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.

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

[v12.x] backport #27224 as semver-patch #27483

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ if (isMainThread) {
const wrapped = mainThreadSetup.wrapProcessMethods(rawMethods);
process.umask = wrapped.umask;
process.chdir = wrapped.chdir;
process.cwd = wrapped.cwd;

// TODO(joyeecheung): deprecate and remove these underscore methods
process._debugProcess = rawMethods._debugProcess;
Expand All @@ -86,11 +87,11 @@ if (isMainThread) {
process.abort = workerThreadSetup.unavailable('process.abort()');
process.chdir = workerThreadSetup.unavailable('process.chdir()');
process.umask = wrapped.umask;
process.cwd = rawMethods.cwd;
}

// Set up methods on the process object for all threads
{
process.cwd = rawMethods.cwd;
process.dlopen = rawMethods.dlopen;
process.uptime = rawMethods.uptime;

Expand Down
15 changes: 14 additions & 1 deletion lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
getEnvMessagePort
} = internalBinding('worker');

const workerIo = require('internal/worker/io');
const {
messageTypes: {
// Messages that may be received by workers
Expand All @@ -38,7 +39,7 @@ const {
STDIO_WANTS_MORE_DATA,
},
kStdioWantsMoreDataCallback
} = require('internal/worker/io');
} = workerIo;

const {
fatalException: originalFatalException
Expand Down Expand Up @@ -89,6 +90,7 @@ if (process.env.NODE_CHANNEL_FD) {
port.on('message', (message) => {
if (message.type === LOAD_SCRIPT) {
const {
cwdCounter,
filename,
doEval,
workerData,
Expand All @@ -111,6 +113,17 @@ port.on('message', (message) => {
publicWorker.parentPort = publicPort;
publicWorker.workerData = workerData;

// The counter is only passed to the workers created by the main thread, not
// to workers created by other workers.
let cachedCwd = '';
const originalCwd = process.cwd;

process.cwd = function() {
cachedCwd = originalCwd();
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we have a proper convention around this, but I'd add a comment explaining this is only for v12.x to reduce conflicts - this is where the difference between the two branches lie so it'll create conflicts when future commits touch here anyway.

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 struggle finding brief words describing this. I could just add // Backwards compatibility patch: removed code to prevent breaking change..

I personally do not think it would be bad without the comment, since we just work on master and people backporting code should see the compatibility commit.

return cachedCwd;
};
workerIo.sharedCwdCounter = cwdCounter;

if (!hasStdin)
process.stdin.push(null);

Expand Down
16 changes: 14 additions & 2 deletions lib/internal/process/main_thread_only.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,15 @@ const { signals } = internalBinding('constants').os;

// The execution of this function itself should not cause any side effects.
function wrapProcessMethods(binding) {
// Cache the working directory to prevent lots of lookups. If the working
// directory is changed by `chdir`, it'll be updated.
let cachedCwd = '';

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

function umask(mask) {
Expand All @@ -32,9 +38,15 @@ function wrapProcessMethods(binding) {
return binding.umask(mask);
}

function cwd() {
cachedCwd = binding.cwd();
Copy link
Member

Choose a reason for hiding this comment

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

Similarly I'd add a comment here.

return cachedCwd;
}

return {
chdir,
umask
umask,
cwd
};
}

Expand Down
19 changes: 17 additions & 2 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

/* global SharedArrayBuffer */

const { Object } = primordials;

const EventEmitter = require('events');
Expand All @@ -16,6 +18,7 @@ const {
const { validateString } = require('internal/validators');
const { getOptionValue } = require('internal/options');

const workerIo = require('internal/worker/io');
const {
drainMessagePort,
MessageChannel,
Expand All @@ -26,8 +29,8 @@ const {
kStdioWantsMoreDataCallback,
setupPortReferencing,
ReadableWorkerStdio,
WritableWorkerStdio,
} = require('internal/worker/io');
WritableWorkerStdio
} = workerIo;
const { deserializeError } = require('internal/error-serdes');
const { pathToFileURL } = require('url');

Expand All @@ -50,6 +53,17 @@ const kParentSideStdio = Symbol('kParentSideStdio');
const SHARE_ENV = Symbol.for('nodejs.worker_threads.SHARE_ENV');
const debug = require('internal/util/debuglog').debuglog('worker');

let cwdCounter;

if (isMainThread) {
cwdCounter = new Uint32Array(new SharedArrayBuffer(4));
const originalChdir = process.chdir;
process.chdir = function(path) {
Atomics.add(cwdCounter, 0, 1);
originalChdir(path);
};
}

class Worker extends EventEmitter {
constructor(filename, options = {}) {
super();
Expand Down Expand Up @@ -131,6 +145,7 @@ class Worker extends EventEmitter {
type: messageTypes.LOAD_SCRIPT,
filename,
doEval: !!options.eval,
cwdCounter: cwdCounter || workerIo.sharedCwdCounter,
workerData: options.workerData,
publicPort: port2,
manifestSrc: getOptionValue('--experimental-policy') ?
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-process-abort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

const common = require('../common');
const assert = require('assert');

if (!common.isMainThread)
common.skip('process.abort() is not available in Workers');

// Check that our built-in methods do not have a prototype/constructor behaviour
// if they don't need to. This could be tested for any of our C++ methods.
assert.strictEqual(process.abort.prototype, undefined);
assert.throws(() => new process.abort(), TypeError);
5 changes: 0 additions & 5 deletions test/parallel/test-process-chdir.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,3 @@ const err = {
};
common.expectsError(function() { process.chdir({}); }, err);
common.expectsError(function() { process.chdir(); }, err);

// Check that our built-in methods do not have a prototype/constructor behaviour
// if they don't need to. This could be tested for any of our C++ methods.
assert.strictEqual(process.cwd.prototype, undefined);
assert.throws(() => new process.cwd(), TypeError);
44 changes: 44 additions & 0 deletions test/parallel/test-worker-process-cwd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';
const assert = require('assert');
const common = require('../common');
const { Worker, isMainThread, parentPort } = require('worker_threads');

// Do not use isMainThread directly, otherwise the test would time out in case
// it's started inside of another worker thread.
if (!process.env.HAS_STARTED_WORKER) {
process.env.HAS_STARTED_WORKER = '1';
if (!isMainThread) {
common.skip('This test can only run as main thread');
}
const w = new Worker(__filename);
process.chdir('..');
w.on('message', common.mustCall((message) => {
assert.strictEqual(message, process.cwd());
process.chdir('..');
w.postMessage(process.cwd());
}));
} else if (!process.env.SECOND_WORKER) {
process.env.SECOND_WORKER = '1';
const firstCwd = process.cwd();
const w = new Worker(__filename);
w.on('message', common.mustCall((message) => {
assert.strictEqual(message, process.cwd());
parentPort.postMessage(firstCwd);
parentPort.onmessage = common.mustCall((obj) => {
const secondCwd = process.cwd();
assert.strictEqual(secondCwd, obj.data);
assert.notStrictEqual(secondCwd, firstCwd);
w.postMessage(secondCwd);
parentPort.unref();
});
}));
} else {
const firstCwd = process.cwd();
parentPort.postMessage(firstCwd);
parentPort.onmessage = common.mustCall((obj) => {
const secondCwd = process.cwd();
assert.strictEqual(secondCwd, obj.data);
assert.notStrictEqual(secondCwd, firstCwd);
process.exit();
});
}