Mitigate WeakMap cycle leak

Summary:
Currently, every `set` operation on a `WeakMap` results in a write to
`self->valueStorage_`, even if the storage was not resized. This means
that a write barrier is executed, and the `valueStorage_` is marked as
a regular cell, instead of going through the special `WeakMap` marking
code in the GC.

This in turn means that if any value in the storage retains a `WeakMap`
key (i.e. a cycle), that cycle will not be collected during a given GC
run. As long as the `WeakMap` is assigned to at least once during each
GC cycle, no such circular references can be collected.

To mitigate this, only write the storage when it is actually resized.
This avoids the write barrier in the common case and allows most
collections to still collect cycles in the `WeakMap`.

This is only a partial fix, since the storage can still be marked
(either through the barrier or when it is in a handle), but it is still
good enough to mitigate most such problems (see #1147).

Reviewed By: tmikov

Differential Revision: D51000231

fbshipit-source-id: 735bac55f19fd9d0a1b58a6d7d77df74c690ab92

lib/VM/JSWeakMapImpl.cpp

@@ -291,6 +291,7 @@ CallResult<uint32_t> JSWeakMapImplBase::getFreeValueStorageIndex(
             ExecutionStatus::EXCEPTION)) {
       return ExecutionStatus::EXCEPTION;
     }
+    self->valueStorage_.setNonNull(runtime, *storageHandle, runtime.getHeap());
   }
 
   // Update internal state here to ensure we don't corrupt it on exception.
@@ -303,8 +304,6 @@ CallResult<uint32_t> JSWeakMapImplBase::getFreeValueStorageIndex(
   }
 
   assert(i < storageHandle->size(runtime) && "invalid index");
-  self->valueStorage_.setNonNull(runtime, *storageHandle, runtime.getHeap());
-
   return i;
 }
 

test/hermes/weakmap-cycle.js

@@ -0,0 +1,18 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes -gc-max-heap=32M %s
+// REQUIRES: !slow_debug
+
+(function () {
+    var foo = new WeakMap();
+
+    for (var i = 0; i < 1000000; i++) {
+        var x = {};
+        foo.set(x, x);
+    }
+})();