[dart2wasm] Fix bug in [Finalizer] implementation

The recent rewrite of [0] kept the existing semantics of the finalizer
implementation but migrated to the new interop semantics.

Turns out in that existing semantics there's a bug, namely that it
casted the peer object of to a non-nullable [Object] which is incorrect.

This CL
* fixes the cast to non-nullable [Object]
* moves the (currently) web-specific tests for
  [Finalizer]/[WeakReference] into `corelib` so it runs on all backends
* expands the finalizer tests to check finalizers get invoked
* expands the weak reference test to ensure weak reference is cleared

[0] https://dart-review.googlesource.com/c/sdk/+/363082/

Issue https://github.com/dart-lang/sdk/issues/55474

Change-Id: Ibd8c186b39100cff9e2f437f1a737034a5364830
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363581
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

pkg/expect/lib/config.dart

@@ -33,5 +33,7 @@ bool get isVmAotConfiguration => _configuration.compiler == Compiler.dartkp;
 
 bool get isVmConfiguration => isVmJitConfiguration || isVmAotConfiguration;
 
+bool get isBrowserConfiguration => _configuration.runtime.isBrowser;
+
 bool get isWebConfiguration =>
     isDart2jsConfiguration || isDart2WasmConfiguration || isDdcConfiguration;

sdk/lib/_internal/wasm/lib/weak_patch.dart

@@ -97,9 +97,9 @@ extension type _JSFinalizationRegistry._(JSObject _) implements JSObject {
   external _JSFinalizationRegistry(JSFunction callback);
   @js_interop.JS('register')
   external void registerWithDetach(ExternalDartReference value,
-      ExternalDartReference peer, ExternalDartReference? detach);
+      ExternalDartReference? peer, ExternalDartReference detach);
   external void register(
-      ExternalDartReference value, ExternalDartReference peer);
+      ExternalDartReference value, ExternalDartReference? peer);
   external void unregister(ExternalDartReference detach);
 }
 
@@ -134,8 +134,8 @@ class _FinalizationRegistryWrapper<T> implements Finalizer<T> {
 
   _FinalizationRegistryWrapper(void Function(T) callback)
       : _jsFinalizationRegistry =
-            _JSFinalizationRegistry(((ExternalDartReference peer) {
-          callback(peer.toDartObject as T);
+            _JSFinalizationRegistry(((ExternalDartReference? peer) {
+          callback(unsafeCast<T>(peer?.toDartObject));
         }).toJS);
 
   void attach(Object value, T peer, {Object? detach}) {
@@ -143,10 +143,10 @@ class _FinalizationRegistryWrapper<T> implements Finalizer<T> {
     if (detach != null) {
       _checkValidWeakTarget(detach);
       _jsFinalizationRegistry.registerWithDetach(value.toExternalReference,
-          (peer as Object).toExternalReference, detach.toExternalReference);
+          peer?.toExternalReference, detach.toExternalReference);
     } else {
       _jsFinalizationRegistry.register(
-          value.toExternalReference, (peer as Object).toExternalReference);
+          value.toExternalReference, peer?.toExternalReference);
     }
   }
 

tests/corelib/finalizer_test.dart

@@ -0,0 +1,103 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+import 'package:expect/config.dart';
+
+final invalidObjects = [true, 1, 1.2, 'a'];
+
+main() async {
+  testFinalizerAttachDetachArgumentValidation();
+
+  // This test doesn't work reliably on the web yet as it's hard to trigger GC
+  // that will run finalizer and weak reference processing.
+  if (isVmConfiguration) {
+    asyncStart();
+    await testFinalizerInvocation();
+    asyncEnd();
+  }
+}
+
+void testFinalizerAttachDetachArgumentValidation() {
+  final finalizer = Finalizer<Foo?>((Foo? foo) {});
+  final foo = Foo();
+
+  // Tests invalid arguments.
+  for (final invalid in invalidObjects) {
+    // Test invalid targets.
+    Expect.throws(() => finalizer.attach(invalid, foo));
+    Expect.throws(() => finalizer.attach(invalid, null));
+    Expect.throws(() => finalizer.attach(invalid, foo, detach: foo));
+    Expect.throws(() => finalizer.attach(invalid, null, detach: null));
+
+    // Test invalid detach tokens.
+    Expect.throws(() => finalizer.attach(foo, foo, detach: invalid));
+    Expect.throws(() => finalizer.attach(foo, null, detach: invalid));
+
+    Expect.throws(() => finalizer.detach(invalid));
+  }
+
+  final target = Foo();
+
+  // Should not cause errors to attach
+  finalizer.attach(target, foo);
+  finalizer.attach(target, null);
+  finalizer.attach(target, foo, detach: target);
+
+  // Can detach with arbitrary (valid) objects.
+  finalizer.detach(target);
+  finalizer.detach(foo);
+  finalizer.detach({});
+}
+
+Future testFinalizerInvocation() async {
+  final invokedPeers = <Foo>{};
+  final expectedPeersInvoked = <Foo>{};
+  final finalizer = Finalizer<Foo>((Foo peer) {
+    invokedPeers.add(peer);
+    asyncEnd();
+  });
+  {
+    for (int i = 0; i < 10; ++i) {
+      final peer = Foo();
+      finalizer.attach(Foo(), peer, detach: peer);
+      if (i % 3 == 0) {
+        finalizer.detach(peer);
+      } else {
+        asyncStart();
+        expectedPeersInvoked.add(peer);
+      }
+    }
+  }
+  asyncStart();
+  while (invokedPeers.length < expectedPeersInvoked.length) {
+    produceGarbage();
+    await Future.delayed(const Duration(milliseconds: 10));
+  }
+  finalizer.detach(Foo()); // Dummy use of [finalizer] to ensure it's not GCed.
+  for (final peer in expectedPeersInvoked) {
+    Expect.isTrue(invokedPeers.contains(peer));
+  }
+  asyncEnd();
+}
+
+void produceGarbage() {
+  const approximateWordSize = 4;
+
+  List<dynamic> sink = [];
+  for (int i = 0; i < 500; ++i) {
+    final filler = i % 2 == 0 ? 1 : sink;
+    if (i % 250 == 1) {
+      // 2 x 25 MB in old space.
+      sink = List.filled(25 * 1024 * 1024 ~/ approximateWordSize, filler);
+    } else {
+      // 498 x 50 KB in new space
+      sink = List.filled(50 * 1024 ~/ approximateWordSize, filler);
+    }
+  }
+  print(sink.hashCode); // Ensure there's real use of the allocation.
+}
+
+class Foo {}

tests/corelib/weak_reference_test.dart

@@ -0,0 +1,50 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+import 'package:expect/config.dart';
+
+import 'finalizer_test.dart' show invalidObjects, produceGarbage, Foo;
+
+main() async {
+  testWeakReferenceArgumentValidation();
+
+  // This test doesn't work reliably on the web yet as it's hard to trigger GC
+  // that will run finalizer and weak reference processing.
+  if (isVmConfiguration) {
+    asyncStart();
+    await testWeakReferenceWeakness();
+    asyncEnd();
+  }
+}
+
+void testWeakReferenceArgumentValidation() {
+  final foo = Foo();
+  final weakRef = WeakReference(foo);
+  Expect.equals(weakRef.target, foo);
+
+  for (final invalid in invalidObjects) {
+    Expect.throws(() => WeakReference(invalid));
+  }
+}
+
+Future testWeakReferenceWeakness() async {
+  late final WeakReference<Foo> weakReference;
+  {
+    Foo? foo = Foo();
+    weakReference = WeakReference<Foo>(foo);
+    Expect.equals(weakReference.target, foo);
+    foo = null;
+  }
+  asyncStart();
+  while (weakReference.target != null) {
+    produceGarbage();
+    await Future.delayed(const Duration(milliseconds: 10));
+  }
+  Expect.isNull(weakReference.target);
+  asyncEnd();
+}

tests/web/wasm_js_shared/weak_reference_test.dart

@@ -1,55 +0,0 @@
-// Copyright (c) 2022, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-// Basic tests for the WeakReference and Finalizer API for the web backends.
-// Does not trigger garbage collection to heavily test the functionality.
-
-import 'package:expect/minitest.dart';
-
-class Foo {
-  int close() {
-    return 42; // no-op, dropped return value.
-  }
-}
-
-void callback(Foo f) {
-  f.close();
-}
-
-main() {
-  test('weak reference', () {
-    var list = ["foo"];
-    var weakRef = WeakReference<List<String>>(list);
-    expect(weakRef.target, equals(list));
-
-    // JavaScript API throws when the representation of target is not 'object'
-    // in the compiled JavaScript.
-    expect(() => WeakReference<String>("foo"), throws);
-    expect(() => WeakReference<int>(1), throws);
-  });
-
-  test('finalizer', () {
-    var finalizer = Finalizer<Foo>(callback);
-    var list = ["foo"];
-    var foo = Foo();
-    // Should not cause errors to attach or detach
-    finalizer.attach(list, foo);
-    finalizer.attach(list, foo, detach: list);
-    finalizer.detach(list);
-
-    // Should not cause errors to use a different detach token
-    var detachList = [1, 2, 3];
-    finalizer.attach(list, foo, detach: detachList);
-    finalizer.detach(detachList);
-
-    // JavaScript API returns false when unknown target detached.
-    // Should not cause an error to detach unknown token.
-    var unknownList = [2, 4, 6];
-    finalizer.detach(unknownList);
-
-    // JavaScript API throws when target and detach token are not objects.
-    expect(() => finalizer.attach("token string value", foo), throws);
-    expect(() => finalizer.detach("detach string value"), throws);
-  });
-}