Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JSC] Implement Symbols as WeakMap keys
https://bugs.webkit.org/show_bug.cgi?id=243483

Reviewed by Ross Kirsling and Saam Barati.

This patch implements stage-3 proposal "Symbols as WeakMap keys"[1].
Previously, WeakMap, WeakSet, FinalizationRegistry, and WeakRef are only accepting objects as keys.
But this proposal extends it to accept non-registered Symbols too.

The runtime implementation is just changed from using JSObject* to JSCell*. And we check whether the
given value is appropriate by using canBeHeldWeakly function (specified in the spec) instead of `isObject()`.

A bit complicated part is DFG / FTL handling for WeakMap / WeakSet functions. We extend it to accept SymbolUse
and CellUse. And we emit appropriate code for that, and modifying clobberizing rules and doesGC rules too for
these new UseKind.

[1]: https://github.com/tc39/proposal-symbols-as-weakmap-keys

* JSTests/stress/finalization-registry-registered-symbol.js: Added.
(shouldThrow):
* JSTests/stress/v8-cleanup-from-different-realm-symbol.js: Added.
(let.timeout_func):
* JSTests/stress/v8-cleanup-proxy-from-different-realm-symbol.js: Added.
(let.timeout_func):
* JSTests/stress/v8-finalization-registry-basics-symbol.js: Copied from JSTests/stress/v8-finalization-registry-basics.js.
(TestConstructFinalizationRegistry):
(TestFinalizationRegistryConstructorCallAsFunction):
(TestConstructFinalizationRegistryCleanupNotCallable):
(TestConstructFinalizationRegistryWithNonCallableProxyAsCleanup):
(TestRegisterTargetAndHoldingsSameValue):
(TestRegisterWithoutFinalizationRegistry):
(TestUnregisterWithNonExistentKey):
(TestUnregisterWithNonFinalizationRegistry):
(TestWeakRefConstructorWithNonObject):
* JSTests/stress/v8-finalization-registry-basics.js:
(TestWeakRefConstructorWithNonObject):
* JSTests/stress/v8-finalizationregistry-and-weakref-symbol.js: Added.
(let.cleanup):
(setTimeout):
* JSTests/stress/v8-finalizationregistry-keeps-holdings-alive-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-finalizationregistry-scheduled-for-cleanup-multiple-times-symbol.js: Added.
(let.cleanup0):
(let.cleanup1):
(let.timeout_func):
* JSTests/stress/v8-multiple-dirty-finalization-registries-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-reentrant-gc-from-cleanup-symbol.js: Added.
(let.reentrant_gc):
(setTimeout):
* JSTests/stress/v8-stress-finalizationregistry-dirty-enqueue-symbol.js: Added.
(i.registries.push.new.FinalizationRegistry):
(registries.forEach):
* JSTests/stress/v8-undefined-holdings-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-unregister-after-cleanup-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-unregister-before-cleanup-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-unregister-called-twice-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-unregister-inside-cleanup2-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-unregister-inside-cleanup3-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-unregister-many-symbol.js: Added.
(let.cleanup):
(let.timeout_func):
* JSTests/stress/v8-weak-unregistertoken-symbol.js: Added.
(FR.new.FinalizationRegistry):
(tryAgain):
* JSTests/stress/weak-map-constructor.js:
* JSTests/stress/weak-map-registered-symbol.js: Added.
(shouldBe):
(shouldThrow):
* JSTests/stress/weak-map-symbol.js: Added.
(shouldBe):
(test):
* JSTests/stress/weak-map-various-one-site.js: Added.
(shouldThrow):
(getTest):
(hasTest):
(setTest):
(deleteTest):
(i.test.set shouldBe):
(i.test):
* JSTests/stress/weak-map-various.js: Added.
(shouldBe):
* JSTests/stress/weak-ref-registered-symbol.js: Added.
(shouldThrow):
* JSTests/stress/weak-set-constructor.js:
* JSTests/stress/weak-set-registered-symbol.js: Added.
(shouldBe):
(shouldThrow):
(test.set add):
* JSTests/stress/weak-set-symbol.js: Added.
(shouldBe):
* JSTests/stress/weak-set-various-one-site.js: Added.
(shouldThrow):
(set shouldThrow):
* JSTests/stress/weak-set-various.js: Added.
(shouldBe):
* JSTests/test262/config.yaml:
* LayoutTests/js/dom/basic-weakmap-expected.txt:
* LayoutTests/js/dom/basic-weakset-expected.txt:
* Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsicCall):
* Source/JavaScriptCore/dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* Source/JavaScriptCore/dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* Source/JavaScriptCore/dfg/DFGOperations.cpp:
(JSC::DFG::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/dfg/DFGOperations.h:
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* Source/JavaScriptCore/runtime/FinalizationRegistryPrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/JSFinalizationRegistry.cpp:
(JSC::JSFinalizationRegistry::registerTarget):
(JSC::JSFinalizationRegistry::unregister):
* Source/JavaScriptCore/runtime/JSFinalizationRegistry.h:
* Source/JavaScriptCore/runtime/JSWeakMap.h:
* Source/JavaScriptCore/runtime/JSWeakMapInlines.h:
(JSC::JSWeakMap::set):
* Source/JavaScriptCore/runtime/JSWeakObjectRef.cpp:
(JSC::JSWeakObjectRef::finishCreation):
* Source/JavaScriptCore/runtime/JSWeakObjectRef.h:
* Source/JavaScriptCore/runtime/WeakMapConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/WeakMapImpl.cpp:
(JSC::WeakMapImpl<WeakMapBucket>::takeSnapshotInternal):
(JSC::WeakMapImpl<WeakMapBucket<WeakMapBucketDataKey>>::takeSnapshot):
(JSC::WeakMapImpl<WeakMapBucket<WeakMapBucketDataKeyValue>>::takeSnapshot):
* Source/JavaScriptCore/runtime/WeakMapImpl.h:
(JSC::WeakMapBucket::setKey):
(JSC::WeakMapBucket::key const):
(JSC::WeakMapBucket::deletedKey):
(JSC::WeakMapImpl::get):
(JSC::WeakMapImpl::findBucket):
(JSC::WeakMapImpl::has):
(JSC::WeakMapImpl::remove):
(JSC::WeakMapImpl::canUseBucket):
(JSC::WeakMapImpl::addInternal):
(JSC::WeakMapImpl::findBucketAlreadyHashed):
* Source/JavaScriptCore/runtime/WeakMapImplInlines.h:
(JSC::jsWeakMapHash):
(JSC::canBeHeldWeakly):
(JSC::WeakMapImpl<WeakMapBucket>::add):
* Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:
(JSC::WeakMapPrototype::finishCreation):
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/WeakMapPrototype.h:
* Source/JavaScriptCore/runtime/WeakObjectRefConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/WeakSetConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:
(JSC::WeakSetPrototype::finishCreation):
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/WeakSetPrototype.h:

Canonical link: https://commits.webkit.org/253135@main
  • Loading branch information
Constellation committed Aug 4, 2022
1 parent 027340c commit 290c0b9
Show file tree
Hide file tree
Showing 60 changed files with 1,631 additions and 132 deletions.
29 changes: 29 additions & 0 deletions JSTests/stress/finalization-registry-registered-symbol.js
@@ -0,0 +1,29 @@
function shouldThrow(func, errorMessage) {
var errorThrown = false;
var error = null;
try {
func();
} catch (e) {
errorThrown = true;
error = e;
}
if (!errorThrown)
throw new Error('not thrown');
if (String(error) !== errorMessage)
throw new Error(`bad error: ${String(error)}`);
}

shouldThrow(() => {
let registry = new FinalizationRegistry(function() { });
registry.register(Symbol.for("Hey"));
}, `TypeError: register requires an object or a non-registered symbol as the target`);

shouldThrow(() => {
let registry = new FinalizationRegistry(function() { });
registry.register(Symbol(), null, Symbol.for("Hey"));
}, `TypeError: register requires an object or a non-registered symbol as the unregistration token`);

shouldThrow(() => {
let registry = new FinalizationRegistry(function() { });
registry.unregister(Symbol.for("Hey"));
}, `TypeError: unregister requires an object or a non-registered symbol as the unregistration token`);
36 changes: 36 additions & 0 deletions JSTests/stress/v8-cleanup-from-different-realm-symbol.js
@@ -0,0 +1,36 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --harmony-weak-refs --expose-gc --noincremental-marking

load("./resources/v8-mjsunit.js", "caller relative");
let Realm = { create: $.createRealm, eval: (r, s) => $.evalScript.call(r, s) };

let r = Realm.create();

let cleanup = Realm.eval(r, "var stored_global; function cleanup() { stored_global = globalThis; } cleanup;");
let realm_global_this = Realm.eval(r, "globalThis");

let fg = new FinalizationRegistry(cleanup);

// Create an symbol and a register it in the FinalizationRegistry. The symbol needs
// to be inside a closure so that we can reliably kill them!

(function() {
for (let i = 0; i < 1000; ++i) {
let symbol = Symbol();
fg.register(symbol);
}
})();

gc();

// Assert that the cleanup function was called in its Realm.
let timeout_func = function() {
let stored_global = Realm.eval(r, "stored_global;");
assertNotEquals(stored_global, globalThis);
assertEquals(stored_global, realm_global_this);
}

setTimeout(timeout_func, 0);
36 changes: 36 additions & 0 deletions JSTests/stress/v8-cleanup-proxy-from-different-realm-symbol.js
@@ -0,0 +1,36 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --harmony-weak-refs --expose-gc --noincremental-marking

load("./resources/v8-mjsunit.js", "caller relative");
let Realm = { create: $.createRealm, eval: (r, s) => $.evalScript.call(r, s) };

let r = Realm.create();

let cleanup = Realm.eval(r, "var stored_global; let cleanup = new Proxy(function() { stored_global = globalThis;}, {}); cleanup");
let realm_global_this = Realm.eval(r, "globalThis");

let fg = new FinalizationRegistry(cleanup);

// Create an object and register it in the FinalizationRegistry. The object needs
// to be inside a closure so that we can reliably kill them!

(function() {
for (let i = 0; i < 1000; ++i) {
let symbol = Symbol();
fg.register(symbol, "holdings");
}
})();

gc();

// Assert that the cleanup function was called in its Realm.
let timeout_func = function() {
let stored_global = Realm.eval(r, "stored_global;");
assertNotEquals(stored_global, globalThis);
assertEquals(stored_global, realm_global_this);
}

setTimeout(timeout_func, 0);
140 changes: 140 additions & 0 deletions JSTests/stress/v8-finalization-registry-basics-symbol.js
@@ -0,0 +1,140 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --harmony-weak-refs

load("./resources/v8-mjsunit.js", "caller relative");

(function TestConstructFinalizationRegistry() {
let fg = new FinalizationRegistry(() => {});
assertEquals(fg.toString(), "[object FinalizationRegistry]");
assertNotSame(fg.__proto__, Object.prototype);
assertSame(fg.__proto__.__proto__, Object.prototype);
})();

(function TestFinalizationRegistryConstructorCallAsFunction() {
let caught = false;
let message = "";
try {
let f = FinalizationRegistry(() => {});
} catch (e) {
message = e.message;
caught = true;
} finally {
assertTrue(caught);
}
})();

(function TestConstructFinalizationRegistryCleanupNotCallable() {
let message = "FinalizationRegistry: cleanup must be callable";
assertThrows(() => { let fg = new FinalizationRegistry(); }, TypeError, message);
assertThrows(() => { let fg = new FinalizationRegistry(1); }, TypeError, message);
assertThrows(() => { let fg = new FinalizationRegistry(null); }, TypeError, message);
})();

(function TestConstructFinalizationRegistryWithCallableProxyAsCleanup() {
let handler = {};
let obj = () => {};
let proxy = new Proxy(obj, handler);
let fg = new FinalizationRegistry(proxy);
})();

(function TestConstructFinalizationRegistryWithNonCallableProxyAsCleanup() {
let message = "FinalizationRegistry: cleanup must be callable";
let handler = {};
let obj = {};
let proxy = new Proxy(obj, handler);
assertThrows(() => { let fg = new FinalizationRegistry(proxy); }, TypeError, message);
})();

(function TestRegisterWithNonObjectTarget() {
let fg = new FinalizationRegistry(() => {});
let message = "FinalizationRegistry.prototype.register: target must be an object";
assertThrows(() => fg.register(1, "holdings"), TypeError, message);
assertThrows(() => fg.register(false, "holdings"), TypeError, message);
assertThrows(() => fg.register("foo", "holdings"), TypeError, message);
assertThrows(() => fg.register(null, "holdings"), TypeError, message);
assertThrows(() => fg.register(undefined, "holdings"), TypeError, message);
})();

(function TestRegisterWithProxy() {
let handler = {};
let obj = {};
let proxy = new Proxy(obj, handler);
let fg = new FinalizationRegistry(() => {});
fg.register(proxy);
})();

(function TestRegisterTargetAndHoldingsSameValue() {
let fg = new FinalizationRegistry(() => {});
let sym = Symbol();
// SameValue(target, holdings) not ok
assertThrows(() => fg.register(sym, sym), TypeError,
"FinalizationRegistry.prototype.register: target and holdings must not be same");
let holdings = Symbol();
fg.register(sym, holdings);
})();

(function TestRegisterWithoutFinalizationRegistry() {
assertThrows(() => FinalizationRegistry.prototype.register.call({}, Symbol(), "holdings"), TypeError);
// Does not throw:
let fg = new FinalizationRegistry(() => {});
FinalizationRegistry.prototype.register.call(fg, Symbol(), "holdings");
})();

(function TestUnregisterWithNonExistentKey() {
let fg = new FinalizationRegistry(() => {});
let success = fg.unregister(Symbol());
assertFalse(success);
})();

(function TestUnregisterWithNonFinalizationRegistry() {
assertThrows(() => FinalizationRegistry.prototype.unregister.call({}, {}),
TypeError);
})();

(function TestUnregisterWithNonObjectUnregisterToken() {
let fg = new FinalizationRegistry(() => {});
assertThrows(() => fg.unregister(1), TypeError);
assertThrows(() => fg.unregister(BigInt(1)), TypeError);
assertThrows(() => fg.unregister('one'), TypeError);
assertThrows(() => fg.unregister(true), TypeError);
assertThrows(() => fg.unregister(false), TypeError);
assertThrows(() => fg.unregister(undefined), TypeError);
assertThrows(() => fg.unregister(null), TypeError);
})();

(function TestWeakRefConstructor() {
let wr = new WeakRef(Symbol());
assertEquals(wr.toString(), "[object WeakRef]");
assertNotSame(wr.__proto__, Object.prototype);

let deref_desc = Object.getOwnPropertyDescriptor(wr.__proto__, "deref");
assertEquals(true, deref_desc.configurable);
assertEquals(false, deref_desc.enumerable);
assertEquals("function", typeof deref_desc.value);
})();

(function TestWeakRefConstructorWithNonObject() {
let message = "WeakRef: target must be an object";
assertThrows(() => new WeakRef(), TypeError, message);
assertThrows(() => new WeakRef(1), TypeError, message);
assertThrows(() => new WeakRef(false), TypeError, message);
assertThrows(() => new WeakRef("foo"), TypeError, message);
assertThrows(() => new WeakRef(null), TypeError, message);
assertThrows(() => new WeakRef(undefined), TypeError, message);
})();

(function TestWeakRefConstructorCallAsFunction() {
let caught = false;
let message = "";
try {
let f = WeakRef(Symbol());
} catch (e) {
message = e.message;
caught = true;
} finally {
assertTrue(caught);
}
})();
3 changes: 0 additions & 3 deletions JSTests/stress/v8-finalization-registry-basics.js
Expand Up @@ -54,7 +54,6 @@ load("./resources/v8-mjsunit.js", "caller relative");
assertThrows(() => fg.register(1, "holdings"), TypeError, message);
assertThrows(() => fg.register(false, "holdings"), TypeError, message);
assertThrows(() => fg.register("foo", "holdings"), TypeError, message);
assertThrows(() => fg.register(Symbol(), "holdings"), TypeError, message);
assertThrows(() => fg.register(null, "holdings"), TypeError, message);
assertThrows(() => fg.register(undefined, "holdings"), TypeError, message);
})();
Expand Down Expand Up @@ -100,7 +99,6 @@ load("./resources/v8-mjsunit.js", "caller relative");
assertThrows(() => fg.unregister(1), TypeError);
assertThrows(() => fg.unregister(BigInt(1)), TypeError);
assertThrows(() => fg.unregister('one'), TypeError);
assertThrows(() => fg.unregister(Symbol()), TypeError);
assertThrows(() => fg.unregister(true), TypeError);
assertThrows(() => fg.unregister(false), TypeError);
assertThrows(() => fg.unregister(undefined), TypeError);
Expand All @@ -124,7 +122,6 @@ load("./resources/v8-mjsunit.js", "caller relative");
assertThrows(() => new WeakRef(1), TypeError, message);
assertThrows(() => new WeakRef(false), TypeError, message);
assertThrows(() => new WeakRef("foo"), TypeError, message);
assertThrows(() => new WeakRef(Symbol()), TypeError, message);
assertThrows(() => new WeakRef(null), TypeError, message);
assertThrows(() => new WeakRef(undefined), TypeError, message);
})();
Expand Down
45 changes: 45 additions & 0 deletions JSTests/stress/v8-finalizationregistry-and-weakref-symbol.js
@@ -0,0 +1,45 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --harmony-weak-refs --expose-gc --noincremental-marking

load("./resources/v8-mjsunit.js", "caller relative");

let cleanup_called = false;
let cleanup = function(holdings) {
let holdings_list = [];
holdings_list.push(holdings);
assertEquals(1, holdings_list.length);
assertEquals("holdings", holdings_list[0]);
cleanup_called = true;
}

let fg = new FinalizationRegistry(cleanup);
let weak_refs = [];
(function() {
for (let i = 0; i < 1000; ++i) {
let s = Symbol();
weak_refs.push(new WeakRef(s));
fg.register(s, "holdings");
}
})();

// Since the WeakRef was created during this turn, it is not cleared by GC. The
// pointer inside the FinalizationRegistry is not cleared either, since the WeakRef
// keeps the target object alive.
gc();
(function() {
weak_refs.forEach((weak_ref) => assertEquals("symbol", typeof(weak_ref.deref())));
})();

// Trigger gc in next task
setTimeout(() => {
gc();

// Check that cleanup callback was called in a follow up task
setTimeout(() => {
assertTrue(cleanup_called);
assertTrue(weak_refs.some((weak_ref) => weak_ref.deref() === undefined));
}, 0);
}, 0);
@@ -0,0 +1,45 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --harmony-weak-refs --expose-gc --noincremental-marking

load("./resources/v8-mjsunit.js", "caller relative");

let cleanup_called = false;
let holdings_list = [];
let cleanup = function(holdings) {
assertFalse(cleanup_called);
holdings_list.push(holdings);
cleanup_called = true;
}

let fg = new FinalizationRegistry(cleanup);
let s1 = Symbol();
let holdings = {'a': 'this is the holdings object'};

// Ignition holds references to objects in temporary registers. These will be
// released when the function exits. So only access o inside a function to
// prevent any references to objects in temporary registers when a gc is
// triggered.
(() => {fg.register(s1, holdings);})()

gc();
assertFalse(cleanup_called);

// Drop the last references to s1.
(() => {s1 = null;})()

// Drop the last reference to the holdings. The FinalizationRegistry keeps it
// alive, so the cleanup function will be called as normal.
holdings = null;
gc();
assertFalse(cleanup_called);

let timeout_func = function() {
assertTrue(cleanup_called);
assertEquals(holdings_list.length, 1);
assertEquals(holdings_list[0].a, "this is the holdings object");
}

setTimeout(timeout_func, 0);

0 comments on commit 290c0b9

Please sign in to comment.