Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: move initialization of APIs for changing process state #31172

Closed
wants to merge 2 commits into from
Closed

lib: move initialization of APIs for changing process state #31172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c13bb0f
lib: move initialization of APIs for changing process state
addaleax Jan 3, 2020
f749556
fixup! lib: move initialization of APIs for changing process state
addaleax Jan 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions lib/internal/bootstrap/switches/does_not_own_process_state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
'use strict';

const credentials = internalBinding('credentials');
const rawMethods = internalBinding('process_methods');
const {
unavailable
} = require('internal/process/worker_thread_only');

process.abort = unavailable('process.abort()');
process.chdir = unavailable('process.chdir()');
process.umask = wrappedUmask;
process.cwd = rawMethods.cwd;

if (credentials.implementsPosixCredentials) {
// TODO: this should be detached from ERR_WORKER_UNSUPPORTED_OPERATION
addaleax marked this conversation as resolved.
Show resolved Hide resolved
const { unavailable } = require('internal/process/worker_thread_only');

process.initgroups = unavailable('process.initgroups()');
process.setgroups = unavailable('process.setgroups()');
process.setegid = unavailable('process.setegid()');
Expand All @@ -16,3 +23,16 @@ if (credentials.implementsPosixCredentials) {

// ---- keep the attachment of the wrappers above so that it's easier to ----
// ---- compare the setups side-by-side -----

const {
codes: { ERR_WORKER_UNSUPPORTED_OPERATION }
} = require('internal/errors');

function wrappedUmask(mask) {
// process.umask() is a read-only operation in workers.
if (mask !== undefined) {
throw new ERR_WORKER_UNSUPPORTED_OPERATION('Setting process.umask()');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this reads Setting process.umask() is not supported in workers, apart from that we need to get rid of the worker part from the message, maybe calling process.umask() as a setter would be less ambiguous (I thought it was about process.umask = something from the first glance)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be changed before landing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think so – it’s unrelated to the changes here.

}

return rawMethods.umask(mask);
}
35 changes: 35 additions & 0 deletions lib/internal/bootstrap/switches/does_own_process_state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
'use strict';

const credentials = internalBinding('credentials');
const rawMethods = internalBinding('process_methods');

process.abort = rawMethods.abort;
process.umask = wrappedUmask;
process.chdir = wrappedChdir;
process.cwd = wrappedCwd;

if (credentials.implementsPosixCredentials) {
const wrapped = wrapPosixCredentialSetters(credentials);
Expand All @@ -16,6 +22,11 @@ if (credentials.implementsPosixCredentials) {
// ---- keep the attachment of the wrappers above so that it's easier to ----
// ---- compare the setups side-by-side -----

const {
parseMode,
validateString
} = require('internal/validators');

function wrapPosixCredentialSetters(credentials) {
const {
ArrayIsArray,
Expand Down Expand Up @@ -94,3 +105,27 @@ function wrapPosixCredentialSetters(credentials) {
setuid: wrapIdSetter('User', _setuid)
};
}

// Cache the working directory to prevent lots of lookups. If the working
// directory is changed by `chdir`, it'll be updated.
let cachedCwd = '';

function wrappedChdir(directory) {
validateString(directory, 'directory');
rawMethods.chdir(directory);
// Mark cache that it requires an update.
cachedCwd = '';
}

function wrappedUmask(mask) {
if (mask !== undefined) {
mask = parseMode(mask, 'mask');
}
return rawMethods.umask(mask);
}

function wrappedCwd() {
if (cachedCwd === '')
cachedCwd = rawMethods.cwd();
return cachedCwd;
}
33 changes: 0 additions & 33 deletions lib/internal/bootstrap/switches/is_main_thread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
const { ObjectDefineProperty } = primordials;
const rawMethods = internalBinding('process_methods');

process.abort = rawMethods.abort;
process.umask = wrappedUmask;
process.chdir = wrappedChdir;
process.cwd = wrappedCwd;

// TODO(joyeecheung): deprecate and remove these underscore methods
process._debugProcess = rawMethods._debugProcess;
process._debugEnd = rawMethods._debugEnd;
Expand Down Expand Up @@ -38,34 +33,6 @@ process.on('removeListener', stopListeningIfSignal);
// ---- compare the setups side-by-side -----

const { guessHandleType } = internalBinding('util');
const {
parseMode,
validateString
} = require('internal/validators');

// Cache the working directory to prevent lots of lookups. If the working
// directory is changed by `chdir`, it'll be updated.
let cachedCwd = '';

function wrappedChdir(directory) {
validateString(directory, 'directory');
rawMethods.chdir(directory);
// Mark cache that it requires an update.
cachedCwd = '';
}

function wrappedUmask(mask) {
if (mask !== undefined) {
mask = parseMode(mask, 'mask');
}
return rawMethods.umask(mask);
}

function wrappedCwd() {
if (cachedCwd === '')
cachedCwd = rawMethods.cwd();
return cachedCwd;
}

function createWritableStdioStream(fd) {
let stream;
Expand Down
21 changes: 0 additions & 21 deletions lib/internal/bootstrap/switches/is_not_main_thread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
'use strict';

const { ObjectDefineProperty } = primordials;
const rawMethods = internalBinding('process_methods');
const {
unavailable
} = require('internal/process/worker_thread_only');

process.abort = unavailable('process.abort()');
process.chdir = unavailable('process.chdir()');
process.umask = wrappedUmask;
process.cwd = rawMethods.cwd;

delete process._debugProcess;
delete process._debugEnd;
Expand Down Expand Up @@ -42,9 +33,6 @@ process.removeListener('removeListener', stopListeningIfSignal);
const {
createWorkerStdio
} = require('internal/worker/io');
const {
codes: { ERR_WORKER_UNSUPPORTED_OPERATION }
} = require('internal/errors');

let workerStdio;
function lazyWorkerStdio() {
Expand All @@ -57,12 +45,3 @@ function getStdout() { return lazyWorkerStdio().stdout; }
function getStderr() { return lazyWorkerStdio().stderr; }

function getStdin() { return lazyWorkerStdio().stdin; }

function wrappedUmask(mask) {
// process.umask() is a read-only operation in workers.
if (mask !== undefined) {
throw new ERR_WORKER_UNSUPPORTED_OPERATION('Setting process.umask()');
}

return rawMethods.umask(mask);
}