From 2354149915b39c383a416bfad5ccdba0ecc9127a Mon Sep 17 00:00:00 2001 From: Mathieu Hofman <86499+mhofman@users.noreply.github.com> Date: Fri, 10 May 2019 09:20:51 +0000 Subject: [PATCH] Schedule node finalized cleanup on a new task Workaround for https://github.com/node-ffi-napi/weak-napi/issues/17 Rewrite node stub to be fully compliant --- src/node/index.ts | 2 +- src/node/stub.test.ts | 23 ++++ src/node/stub.ts | 161 +++++++++++++++++++++++--- src/tests/FinalizationGroup.shared.ts | 6 +- 4 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 src/node/stub.test.ts diff --git a/src/node/index.ts b/src/node/index.ts index 38ca8e2..86000e4 100644 --- a/src/node/index.ts +++ b/src/node/index.ts @@ -22,7 +22,7 @@ const observedInfos = new Set(); function finalizedCallback(this: ObjectInfo) { if (!observedInfos.has(this)) return; observedInfos.delete(this); - finalizationGroupJobs.setFinalized(this); + setImmediate(() => finalizationGroupJobs.setFinalized(this)); } function getInfo(target: object) { diff --git a/src/node/stub.test.ts b/src/node/stub.test.ts new file mode 100644 index 0000000..de9714a --- /dev/null +++ b/src/node/stub.test.ts @@ -0,0 +1,23 @@ +import { describe } from "../../tests/setup.js"; +import { shouldBehaveAsFinalizationGroupAccordingToSpec } from "../tests/FinalizationGroup.shared.js"; +import { shouldBehaveAsWeakRefAccordingToSpec } from "../tests/WeakRef.shared.js"; +import { gc, gcAvailable } from "../../tests/collector-helper.js"; +import available from "./available.js"; + +if (available) + describe("Weakrefs node stub", function() { + const shimDetails = import("./stub.js").then(exports => ({ + gc, + ...exports, + })); + + shouldBehaveAsWeakRefAccordingToSpec( + shimDetails, + gcAvailable, + !available + ); + shouldBehaveAsFinalizationGroupAccordingToSpec( + shimDetails, + gcAvailable + ); + }); diff --git a/src/node/stub.ts b/src/node/stub.ts index 762d69c..cb6b28b 100644 --- a/src/node/stub.ts +++ b/src/node/stub.ts @@ -3,46 +3,170 @@ import WeakTag from "@mhofman/weak-napi-native/weak-tag.js"; // @ts-ignore import ObjectInfo from "@mhofman/weak-napi-native/object-info.js"; +import { FinalizationGroup, WeakRef } from "../weakrefs.js"; +import isObject from "../utils/lodash/isObject.js"; +import { setImmediate } from "../utils/tasks/setImmediate.js"; + +type Cell = { + holdings: any; + unregisterToken: object | undefined; +}; + const map = new WeakMap>(); -import { FinalizationGroup, WeakRef } from "../weakrefs.js"; +const cellsForGroup = new WeakMap< + FinalizationGroup, + Map +>(); + +const registrations = new WeakMap>(); + +function getCells(group: FinalizationGroup) { + const cells = cellsForGroup.get(group); + if (!cells) throw new TypeError(); + return cells; +} + +type Entries = Iterable<[ObjectInfo, Cell]>; + +function* getEmptyCellEntries(context: { + cells: Map; +}): Entries { + for (const [info, cell] of context.cells.entries()) { + if (!context.cells) throw new TypeError(); + if (info.target) continue; + context.cells.delete(info); + yield [info, cell]; + } +} + +function* getCellEntry(context: { + info: ObjectInfo; + cells: Map; +}): Entries { + const cell = context.cells.get(context.info)!; + context.cells.delete(context.info); + yield [context.info, cell]; +} + +const CleanupIterator: ( + entries: Iterable<[ObjectInfo, Cell]> +) => FinalizationGroup.CleanupIterator = function* CleanupIterator( + entries: Iterable<[ObjectInfo, Cell]> +) { + for (const [info, cell] of entries) { + if (cell.unregisterToken) + registrations.get(cell.unregisterToken)!.delete(info); + yield cell.holdings; + } +} as ( + entries: Iterable<[ObjectInfo, Cell]> +) => FinalizationGroup.CleanupIterator; + +Object.defineProperty( + Object.getPrototypeOf(CleanupIterator.prototype), + Symbol.toStringTag, + { + value: "FinalizationGroup Cleanup Iterator", + configurable: true, + } +); + +class FinalizationGroupNodeStub + implements FinalizationGroup { + private readonly finalizedCallback: ObjectInfo.FinalizedCallback; -// stub FinalizationGroup that calls the callback directly for each -// registered target -// No holding or unregister -class FinalizationGroupNodeStub implements FinalizationGroup { - private finalizedCallback: ObjectInfo.FinalizedCallback; static get [Symbol.species]() { return FinalizationGroupNodeStub; } - constructor(callback: FinalizationGroup.CleanupCallback) { + + constructor( + private readonly cleanupCallback: FinalizationGroup.CleanupCallback< + Holdings + > + ) { + if (typeof cleanupCallback != "function") throw new TypeError(); + const cells = new Map(); + cellsForGroup.set(this, cells); this.finalizedCallback = function(this: ObjectInfo) { - callback(([this] as unknown) as FinalizationGroup.CleanupIterator< - ObjectInfo - >); + setImmediate(() => { + if (!cells.get(this)) return; + const context = { info: this, cells }; + cleanupCallback(CleanupIterator(getCellEntry(context))); + context.info = context.cells = undefined!; + }); }; } + register( target: object, - holdingsIgnored: any, - unregisterTokenIgnored?: any - ): ObjectInfo { + holdings: Holdings, + unregisterToken?: object + ): void { + const cells = getCells(this); + let tagSet = map.get(target); if (!tagSet) { tagSet = new Set(); map.set(target, tagSet); } + let registrationSet = unregisterToken + ? registrations.get(unregisterToken) + : undefined; + if (!registrationSet && unregisterToken !== undefined) { + if (!isObject(unregisterToken)) throw new TypeError(); + registrationSet = new Set(); + registrations.set(unregisterToken, registrationSet); + } const info = new ObjectInfo(target, this.finalizedCallback); tagSet.add(new WeakTag(info)); - return info; + cells.set(info, { + holdings, + unregisterToken, + }); + if (registrationSet) registrationSet.add(info); } - unregister(unregisterToken?: any): boolean { - return false; + unregister(unregisterToken: object): boolean { + const cells = getCells(this); + + const registrationSet = registrations.get(unregisterToken); + if (!registrationSet) { + if (!isObject(unregisterToken)) throw new TypeError(); + return false; + } + + let removed = false; + for (const info of registrationSet) { + const cell = cells.get(info)!; + + if (!cell) continue; + + cells.delete(info); + registrationSet.delete(info); + removed = true; + } + + return removed; } + cleanupSome( - cleanupCallback?: FinalizationGroup.CleanupCallback | undefined - ): void {} + cleanupCallback?: + | FinalizationGroup.CleanupCallback + | undefined + ): void { + const context = { cells: getCells(this) }; + const emptyObjectInfos = getEmptyCellEntries(context); + + if (cleanupCallback === undefined) { + cleanupCallback = this + .cleanupCallback as FinalizationGroup.CleanupCallback; + } + + cleanupCallback(CleanupIterator(emptyObjectInfos)); + + context.cells = undefined!; + } } class WeakRefNodeStub implements WeakRef { @@ -52,6 +176,7 @@ class WeakRefNodeStub implements WeakRef { return WeakRefNodeStub; } constructor(target: T) { + if (!isObject(target)) throw new TypeError(); this.info = new ObjectInfo(target, () => {}); } diff --git a/src/tests/FinalizationGroup.shared.ts b/src/tests/FinalizationGroup.shared.ts index f3c33c9..b249490 100644 --- a/src/tests/FinalizationGroup.shared.ts +++ b/src/tests/FinalizationGroup.shared.ts @@ -522,8 +522,10 @@ export function shouldBehaveAsFinalizationGroupAccordingToSpec( it("doesn't remove cell if iterator is closed before", async function() { const holdings = [{}, {}]; let notIterated: object | undefined; + let invocations = 0; const finalizationGroup = new FinalizationGroup( items => { + invocations++; for (const item of items) { notIterated = item; expect(holdings).to.contain(notIterated); @@ -546,7 +548,9 @@ export function shouldBehaveAsFinalizationGroupAccordingToSpec( object = undefined!; await collected; expect(notIterated).to.be.ok; - if (unregisterReturnsBool) { + if (invocations > 1) { + this.skip(); + } else if (unregisterReturnsBool) { expect(finalizationGroup.unregister(notIterated!)) .to.be.true; } else if (workingCleanupSome) {