Skip to content

Commit 9c98887

Browse files
authored
fix: fix CI with Node.js 18+ MONGOSH-1344 (#38)
This should un-break boxednode when building against Node.js versions that have snapshot support, but not in a way that is compatible with the way in which we embed Node.js.
1 parent 050ca21 commit 9c98887

File tree

4 files changed

+58
-3
lines changed

4 files changed

+58
-3
lines changed

.github/workflows/nodejs.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
fail-fast: false
1414
matrix:
1515
os: [ubuntu-latest, macos-latest]
16-
node-version: [14.x, 16.x, 18.x]
16+
node-version: [14.x, 16.x, 18.x, 19.x]
1717
runs-on: ${{ matrix.os }}
1818
steps:
1919
- uses: actions/checkout@v2
@@ -35,7 +35,7 @@ jobs:
3535
strategy:
3636
fail-fast: false
3737
matrix:
38-
node-version: [14.x, 16.x, 18.x]
38+
node-version: [14.x, 16.x, 18.x, 19.x]
3939
shard: [1, 2, 3]
4040
runs-on: windows-latest
4141
steps:

resources/main-template.cc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,17 @@ using namespace v8;
2424
#define USE_OWN_LEGACY_PROCESS_INITIALIZATION 1
2525
#endif
2626

27+
// 18.1.0 is the current minimum version that has https://github.com/nodejs/node/pull/42809,
28+
// which introduced crashes when using workers, and later 18.9.0 is the current
29+
// minimum version to contain https://github.com/nodejs/node/pull/44252, which
30+
// introcued crashes when using the vm module.
31+
// We should be able to remove this restriction again once Node.js stops relying
32+
// on global state for determining whether snapshots are enabled or not
33+
// (after https://github.com/nodejs/node/pull/45888, hopefully).
34+
#if NODE_VERSION_AT_LEAST(18, 1, 0)
35+
#define PASS_NO_NODE_SNAPSHOT_OPTION 1
36+
#endif
37+
2738
#ifdef USE_OWN_LEGACY_PROCESS_INITIALIZATION
2839
namespace boxednode {
2940
void InitializeOncePerProcess();
@@ -196,8 +207,12 @@ static int BoxednodeMain(std::vector<std::string> args) {
196207
std::vector<std::string> exec_args;
197208
std::vector<std::string> errors;
198209

199-
if (args.size() > 0)
210+
if (args.size() > 0) {
200211
args.insert(args.begin() + 1, "--");
212+
#ifdef PASS_NO_NODE_SNAPSHOT_OPTION
213+
args.insert(args.begin() + 1, "--no-node-snapshot");
214+
#endif
215+
}
201216

202217
// Parse Node.js CLI options, and print any errors that have occurred while
203218
// trying to parse them.

src/index.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,15 @@ async function getNodeSourceForVersion (range: string, dir: string, logger: Logg
147147
return path.join(dir, `node-${version}`);
148148
}
149149

150+
async function getNodeVersionFromSourceDirectory (dir: string): Promise<[number, number, number]> {
151+
const versionFile = await fs.readFile(path.join(dir, 'src', 'node_version.h'), 'utf8');
152+
153+
const major = +versionFile.match(/^#define\s+NODE_MAJOR_VERSION\s+(?<version>\d+)\s*$/m)?.groups?.version;
154+
const minor = +versionFile.match(/^#define\s+NODE_MINOR_VERSION\s+(?<version>\d+)\s*$/m)?.groups?.version;
155+
const patch = +versionFile.match(/^#define\s+NODE_PATCH_VERSION\s+(?<version>\d+)\s*$/m)?.groups?.version;
156+
return [major, minor, patch];
157+
}
158+
150159
// Compile a Node.js build in a given directory from source
151160
async function compileNode (
152161
sourcePath: string,
@@ -163,6 +172,19 @@ async function compileNode (
163172
env: env
164173
};
165174

175+
// Node.js 19.4.0 is currently the minimum version that has https://github.com/nodejs/node/pull/45887.
176+
// We want to disable the shared-ro-heap flag since it would require
177+
// all snapshots used by Node.js to be equal, something that we don't
178+
// want to or need to guarantee as embedders.
179+
const nodeVersion = await getNodeVersionFromSourceDirectory(sourcePath);
180+
if (nodeVersion[0] > 19 || (nodeVersion[0] === 19 && nodeVersion[1] >= 4)) {
181+
if (process.platform !== 'win32') {
182+
buildArgs.unshift('--disable-shared-readonly-heap');
183+
} else {
184+
buildArgs.unshift('no-shared-roheap');
185+
}
186+
}
187+
166188
if (process.platform !== 'win32') {
167189
const configure: string[] = ['./configure', ...buildArgs];
168190
for (const module of linkedJSModules) {

test/index.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,24 @@ describe('basic functionality', () => {
6161
assert.strictEqual(stdout, 'true\n');
6262
}
6363

64+
{
65+
const { stdout } = await execFile(
66+
path.resolve(__dirname, `resources/example${exeSuffix}`), ['require("vm").runInNewContext("21*2")'],
67+
{ encoding: 'utf8' });
68+
assert.strictEqual(stdout, '42\n');
69+
}
70+
71+
{
72+
const { stdout } = await execFile(
73+
path.resolve(__dirname, `resources/example${exeSuffix}`), [
74+
'new (require("worker_threads").Worker)' +
75+
'("require(`worker_threads`).parentPort.postMessage(21*2)", {eval:true})' +
76+
'.once("message", console.log);0'
77+
],
78+
{ encoding: 'utf8' });
79+
assert.strictEqual(stdout, '0\n42\n');
80+
}
81+
6482
if (process.platform !== 'win32') {
6583
const proc = childProcess.spawn(
6684
path.resolve(__dirname, `resources/example${exeSuffix}`),

0 commit comments

Comments
 (0)