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

fix: enable Node.js --expose-gc via require('expose-gc') #3207

Merged
merged 2 commits into from
Jun 2, 2021
Merged
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
2 changes: 1 addition & 1 deletion .github/workflows/test-all-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ jobs:
# END-RESTORE-BOILERPLATE

- name: yarn test (zoe)
timeout-minutes: 20
timeout-minutes: 30
run: cd packages/zoe && yarn test
env:
ESM_DISABLE_CACHE: true
Expand Down
3 changes: 0 additions & 3 deletions packages/SwingSet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@
"files": [
"test/**/test-*.js"
],
"nodeArguments": [
"--expose-gc"
],
"require": [
"esm"
],
Expand Down
7 changes: 4 additions & 3 deletions packages/SwingSet/src/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ import { importBundle } from '@agoric/import-bundle';
import { makeMeteringTransformer } from '@agoric/transform-metering';
import { xsnap, makeSnapstore } from '@agoric/xsnap';

import engineGC from './engine-gc';
import { WeakRef, FinalizationRegistry } from './weakref';
import { startSubprocessWorker } from './spawnSubprocessWorker';
import { waitUntilQuiescent } from './waitUntilQuiescent';
import { gcAndFinalize } from './gc-and-finalize';
import { makeGcAndFinalize } from './gc-and-finalize';
import { insistStorageAPI } from './storageAPI';
import { insistCapData } from './capdata';
import { parseVatSlot } from './parseVatSlots';
Expand Down Expand Up @@ -272,7 +273,7 @@ export async function makeSwingsetController(
const supercode = require.resolve(
'./kernel/vatManager/supervisor-subprocess-node.js',
);
const args = ['--expose-gc', '-r', 'esm', supercode];
const args = ['-r', 'esm', supercode];
return startSubprocessWorker(process.execPath, args);
}

Expand All @@ -299,7 +300,7 @@ export async function makeSwingsetController(
writeSlogObject,
WeakRef,
FinalizationRegistry,
gcAndFinalize,
gcAndFinalize: makeGcAndFinalize(engineGC),
};

const kernelOptions = { verbose };
Expand Down
16 changes: 16 additions & 0 deletions packages/SwingSet/src/engine-gc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* eslint-disable global-require */
/* global globalThis require */
let bestGC = globalThis.gc;
if (typeof bestGC !== 'function') {
// Node.js v8 wizardry.
const v8 = require('v8');
const vm = require('vm');
v8.setFlagsFromString('--expose_gc');
bestGC = vm.runInNewContext('gc');
// Hide the gc global from new contexts/workers.
v8.setFlagsFromString('--no-expose_gc');
}

// Export a const.
const engineGC = bestGC;
export default engineGC;
43 changes: 22 additions & 21 deletions packages/SwingSet/src/gc-and-finalize.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global gc setImmediate */
/* global setImmediate */

/* A note on our GC terminology:
*
Expand Down Expand Up @@ -33,9 +33,10 @@
* The transition from UNREACHABLE to COLLECTED can happen spontaneously, as
* the JS engine decides it wants to perform GC. It will also happen
* deliberately if we provoke a GC call with a magic function like `gc()`
* (when Node.js is run with `--expose-gc`, or when XS is configured to
* provide it as a C-level callback). We can force GC, but we cannot prevent
* it from happening at other times.
* (when Node.js imports `engine-gc`, which is morally-equivalent to
* running with `--expose-gc`, or when XS is configured to provide it as a
* C-level callback). We can force GC, but we cannot prevent it from happening
* at other times.
*
* FinalizationRegistry callbacks are defined to run on their own turn, so
* the transition from COLLECTED to FINALIZED occurs at a turn boundary.
Expand Down Expand Up @@ -63,23 +64,23 @@
* dummy pre-resolved Promise.
*/

let alreadyWarned = false;
export async function gcAndFinalize() {
if (typeof gc !== 'function') {
if (!alreadyWarned) {
alreadyWarned = true;
console.warn(
Error(`no gc() function; disabling deterministic finalizers`),
);
}
return;
export function makeGcAndFinalize(gcPower) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm kinda glad to see that alreadyWarned go away.. I'm not sure how it worked on our unit test (which extracted the source code of gcAndFinalize for evaluating in xsnap, which would lose the let alreadyWarnred = false; line). Maybe it was just relying upon unbound names being treated as properties of the global.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't know how that worked.

if (typeof gcPower !== 'function') {
console.warn(
Error(`no gcPower() function; skipping finalizer provocation`),
);
}
return async function gcAndFinalize() {
if (typeof gcPower !== 'function') {
return;
}

// on Node.js, GC seems to work better if the promise queue is empty first
await new Promise(setImmediate);
// on xsnap, we must do it twice for some reason
await new Promise(setImmediate);
gc();
// this gives finalizers a chance to run
await new Promise(setImmediate);
// on Node.js, GC seems to work better if the promise queue is empty first
await new Promise(setImmediate);
// on xsnap, we must do it twice for some reason
await new Promise(setImmediate);
gcPower();
// this gives finalizers a chance to run
await new Promise(setImmediate);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import '../../types';
import { assert, details as X } from '@agoric/assert';
import { importBundle } from '@agoric/import-bundle';
import { makeMarshal } from '@agoric/marshal';
import engineGC from '../../engine-gc';
import { WeakRef, FinalizationRegistry } from '../../weakref';
import { gcAndFinalize } from '../../gc-and-finalize';
import { makeGcAndFinalize } from '../../gc-and-finalize';
import { waitUntilQuiescent } from '../../waitUntilQuiescent';
import { makeLiveSlots } from '../liveSlots';
import {
Expand Down Expand Up @@ -79,7 +80,7 @@ parentPort.on('message', ([type, ...margs]) => {
WeakRef,
FinalizationRegistry,
waitUntilQuiescent,
gcAndFinalize,
gcAndFinalize: makeGcAndFinalize(engineGC),
});
const ls = makeLiveSlots(
syscall,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import fs from 'fs';
import { assert, details as X } from '@agoric/assert';
import { importBundle } from '@agoric/import-bundle';
import { makeMarshal } from '@agoric/marshal';
import engineGC from '../../engine-gc';
import { WeakRef, FinalizationRegistry } from '../../weakref';
import { gcAndFinalize } from '../../gc-and-finalize';
import { makeGcAndFinalize } from '../../gc-and-finalize';
import { arrayEncoderStream, arrayDecoderStream } from '../../worker-protocol';
import {
netstringEncoderStream,
Expand Down Expand Up @@ -92,7 +93,7 @@ fromParent.on('data', ([type, ...margs]) => {
WeakRef,
FinalizationRegistry,
waitUntilQuiescent,
gcAndFinalize,
gcAndFinalize: makeGcAndFinalize(engineGC),
});
const ls = makeLiveSlots(
syscall,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { makeMarshal } from '@agoric/marshal';
import '../../types';
// grumble... waitUntilQuiescent is exported and closes over ambient authority
import { waitUntilQuiescent } from '../../waitUntilQuiescent';
import { gcAndFinalize } from '../../gc-and-finalize';
import { makeGcAndFinalize } from '../../gc-and-finalize';
import { insistVatDeliveryObject, insistVatSyscallResult } from '../../message';

import { makeLiveSlots } from '../liveSlots';
Expand Down Expand Up @@ -185,7 +185,7 @@ function makeWorker(port) {
WeakRef,
FinalizationRegistry,
waitUntilQuiescent,
gcAndFinalize,
gcAndFinalize: makeGcAndFinalize(globalThis.gc),
});

const ls = makeLiveSlots(
Expand Down
6 changes: 4 additions & 2 deletions packages/SwingSet/test/liveslots-helpers.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import engineGC from '../src/engine-gc';

import { WeakRef, FinalizationRegistry } from '../src/weakref';
import { waitUntilQuiescent } from '../src/waitUntilQuiescent';
import { gcAndFinalize } from '../src/gc-and-finalize';
import { makeGcAndFinalize } from '../src/gc-and-finalize';
import { makeLiveSlots } from '../src/kernel/liveSlots';

export function buildSyscall() {
Expand Down Expand Up @@ -43,7 +45,7 @@ export function makeDispatch(
WeakRef,
FinalizationRegistry,
waitUntilQuiescent,
gcAndFinalize,
gcAndFinalize: makeGcAndFinalize(engineGC),
});
const { setBuildRootObject, dispatch } = makeLiveSlots(
syscall,
Expand Down
31 changes: 13 additions & 18 deletions packages/SwingSet/test/test-gc-and-finalize.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
/* global gc FinalizationRegistry WeakRef */
/* global FinalizationRegistry WeakRef */
// eslint-disable-next-line import/order
import { test } from '../tools/prepare-test-env-ava';

import * as childProcess from 'child_process';
import * as os from 'os';
import { xsnap } from '@agoric/xsnap';
import { gcAndFinalize } from '../src/gc-and-finalize';
import engineGC from '../src/engine-gc';
import { makeGcAndFinalize } from '../src/gc-and-finalize';

test(`have gc() on Node.js`, async t => {
t.is(typeof gc, 'function', 'environment is missing top-level gc()');
// Under Node.js, you must use `node --expose-gc PROGRAM`. Under AVA+Node,
// add `nodeArguments: [ "--expose-gc" ]` to the package.json 'ava:'
// stanza. Under XS, make sure your application (e.g. xsnap) provides a
// `gc` C callback on the global object.
});

function setup() {
function makeVictim() {
const victim = { doomed: 'oh no' };
const finalized = ['finalizer not called'];
const fr = new FinalizationRegistry(_tag => {
Expand All @@ -26,13 +19,15 @@ function setup() {
return { finalized, fr, wr };
}

async function provokeGC() {
// the transition from REACHABLE to UNREACHABLE happens as soon as setup()
async function provokeGC(myGC) {
const gcAndFinalize = makeGcAndFinalize(myGC);

// the transition from REACHABLE to UNREACHABLE happens as soon as makeVictim()
// finishes, and the local 'victim' binding goes out of scope

// we must retain the FinalizationRegistry to let the callback fire
// eslint-disable-next-line no-unused-vars
const { finalized, fr, wr } = setup();
const { finalized, fr, wr } = makeVictim();

// the transition from UNREACHABLE to COLLECTED can happen at any moment,
// but is far more likely to happen if we force it
Expand All @@ -53,7 +48,7 @@ if (
}

ltest(`can provoke gc on Node.js`, async t => {
const { wrState, finalizerState } = await provokeGC();
const { wrState, finalizerState } = await provokeGC(engineGC);
t.is(wrState, 'weakref is dead');
t.is(finalizerState, 'finalizer was called');
});
Expand Down Expand Up @@ -81,10 +76,10 @@ test(`can provoke gc on xsnap`, async t => {
const opts = options();
const vat = xsnap(opts);
const code = `
${gcAndFinalize}
${setup}
${makeGcAndFinalize}
${makeVictim}
${provokeGC}
provokeGC().then(data => issueCommand(ArrayBuffer.fromString(JSON.stringify(data))));
provokeGC(globalThis.gc).then(data => issueCommand(ArrayBuffer.fromString(JSON.stringify(data))));
`;
await vat.evaluate(code);
await vat.close();
Expand Down
5 changes: 4 additions & 1 deletion packages/SwingSet/test/test-liveslots.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import { E } from '@agoric/eventual-send';
import { Far } from '@agoric/marshal';
import { makePromiseKit } from '@agoric/promise-kit';
import { assert, details as X } from '@agoric/assert';
import engineGC from '../src/engine-gc';
import { waitUntilQuiescent } from '../src/waitUntilQuiescent';
import { gcAndFinalize } from '../src/gc-and-finalize';
import { makeGcAndFinalize } from '../src/gc-and-finalize';
import { makeLiveSlots } from '../src/kernel/liveSlots';
import { buildSyscall, makeDispatch } from './liveslots-helpers';
import {
Expand Down Expand Up @@ -323,6 +324,7 @@ test('liveslots retires outbound promise IDs after reject', async t => {
});

test('liveslots retains pending exported promise', async t => {
const gcAndFinalize = makeGcAndFinalize(engineGC);
const { log, syscall } = buildSyscall();
let watch;
const success = [];
Expand Down Expand Up @@ -661,6 +663,7 @@ test('disavow', async t => {
});

test('liveslots retains device nodes', async t => {
const gcAndFinalize = makeGcAndFinalize(engineGC);
const { syscall } = buildSyscall();
let watch;
const recognize = new WeakSet(); // real WeakSet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { test } from '../../tools/prepare-test-env-ava';
// eslint-disable-next-line import/order
import { Far } from '@agoric/marshal';

import { gcAndFinalize } from '../../src/gc-and-finalize';
import engineGC from '../../src/engine-gc';
import { makeGcAndFinalize } from '../../src/gc-and-finalize';
import { makeFakeVirtualObjectManager } from '../../tools/fakeVirtualObjectManager';

// empty object, used as makeWeakStore() key
Expand Down Expand Up @@ -76,6 +77,7 @@ function stashRemotableFour(holderMaker) {
}

test('remotables retained by virtualized data', async t => {
const gcAndFinalize = makeGcAndFinalize(engineGC);
const vomOptions = { cacheSize: 3, weak: true };
const vom = makeFakeVirtualObjectManager(vomOptions);
const { makeWeakStore, makeKind } = vom;
Expand Down
16 changes: 3 additions & 13 deletions packages/SwingSet/test/virtualObjects/test-weakcollections.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/* global __dirname globalThis */
/* global __dirname */
import { test } from '../../tools/prepare-test-env-ava';

// eslint-disable-next-line import/order
import path from 'path';

import engineGC from '../../src/engine-gc';
import { provideHostStorage } from '../../src/hostStorage';
import { initializeSwingset, makeSwingsetController } from '../../src/index';
import { makeFakeVirtualObjectManager } from '../../tools/fakeVirtualObjectManager';
Expand All @@ -17,17 +18,7 @@ function capargs(args, slots = []) {
return capdata(JSON.stringify(args), slots);
}

let gc;
function insistGC(t) {
if (globalThis.gc) {
gc = globalThis.gc;
} else {
t.fail(`GC needs to be enabled for this test to work`);
}
}

test('weakMap in vat', async t => {
insistGC(t);
const config = {
bootstrap: 'bootstrap',
defaultManagerType: 'local',
Expand Down Expand Up @@ -70,7 +61,7 @@ test('weakMap in vat', async t => {
'probe of [object Promise] returns fep',
]);
await doSimple('betweenProbes');
gc();
engineGC();
const postGCResult = await doSimple('runProbes');
t.deepEqual(postGCResult, capargs('probes done'));
t.deepEqual(nextLog(), [
Expand All @@ -86,7 +77,6 @@ test('weakMap in vat', async t => {
});

test('weakMap vref handling', async t => {
insistGC(t);
const log = [];
const {
VirtualObjectAwareWeakMap,
Expand Down
6 changes: 5 additions & 1 deletion packages/agoric-cli/integration-tests/test-workflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,11 @@ test('workflow', async t => {
]);
finalizers.push(() => pkill(deployP.childProcess, 'SIGINT'));

timeout = setTimeout(deployResult.resolve, 120 * 1000, 'timeout');
timeout = setTimeout(
deployResult.resolve,
TIMEOUT_SECONDS * 1000,
'timeout',
);
const done = await Promise.race([deployResult.promise, deployP]);
t.is(done, 0, `deploy successful before timeout`);
clearTimeout(timeout);
Expand Down
7 changes: 1 addition & 6 deletions packages/agoric-cli/lib/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,7 @@ export default async function startMain(progname, rawArgs, powers, opts) {
const SDK_IMAGE = `agoric/agoric-sdk:${opts.dockerTag}`;
const SOLO_IMAGE = `agoric/cosmic-swingset-solo:${opts.dockerTag}`;

const pspawnEnv = {
...process.env,
// TODO: We'd love to --expose-gc in this environment variable,
// but Node.js rejects it.
// NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --expose-gc`,
};
const pspawnEnv = { ...process.env };
const pspawn = makePspawn({ env: pspawnEnv, spawn, log, chalk });

let keysSpawn;
Expand Down
1 change: 0 additions & 1 deletion packages/dapp-svelte-wallet/api/src/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ export function buildRootObject(_vatPowers) {
harden(preapprovedBridge);

async function getWallet(bank) {
console.error('/// importing bank assets', bank);
if (bank) {
walletRoot.importBankAssets(bank);
}
Expand Down
Loading