From 2686f237f8cb0897b369d0efbd3a2d353093690a Mon Sep 17 00:00:00 2001
From: Matt Mastracci <matthew@mastracci.com>
Date: Wed, 15 May 2024 09:33:59 -0600
Subject: [PATCH] fix: Re-enable pointer compression (#1473)

* fix: Re-enable pointer compression

* macos-13

* Add a tight loop test

* Better test

* Update tests/test_api.rs
---
 .gn               |  6 ------
 src/binding.cc    |  8 ++++++++
 tests/test_api.rs | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/.gn b/.gn
index f7097ddf..b90b3821 100644
--- a/.gn
+++ b/.gn
@@ -59,12 +59,6 @@ default_args = {
   # compile.
   v8_enable_verify_heap = false
 
-  # V8 introduced a bug in 11.1 that causes the External Pointer Table to never
-  # be cleaned which causes resource exhaustion. Disabling pointer compression
-  # makes sure that the EPT is not used.
-  # https://bugs.chromium.org/p/v8/issues/detail?id=13640&q=garbage%20collection&can=2
-  v8_enable_pointer_compression = false
-
   # Maglev *should* be supported when pointer compression is disabled as per
   # https://chromium-review.googlesource.com/c/v8/v8/+/4753150, but it still
   # fails to compile.
diff --git a/src/binding.cc b/src/binding.cc
index 82850ec9..9ca3abf0 100644
--- a/src/binding.cc
+++ b/src/binding.cc
@@ -3231,6 +3231,14 @@ void v8__HeapProfiler__TakeHeapSnapshot(v8::Isolate* isolate,
   const_cast<v8::HeapSnapshot*>(snapshot)->Delete();
 }
 
+// This is necessary for v8__internal__GetIsolateFromHeapObject() to be
+// reliable enough for our purposes.
+#if UINTPTR_MAX == 0xffffffffffffffff && \
+    !(defined V8_SHARED_RO_HEAP or defined V8_COMPRESS_POINTERS)
+#error V8 must be built with either the 'v8_enable_pointer_compression' or \
+'v8_enable_shared_ro_heap' feature enabled.
+#endif
+
 v8::Isolate* v8__internal__GetIsolateFromHeapObject(const v8::Data& data) {
   namespace i = v8::internal;
   i::Tagged<i::Object> object(reinterpret_cast<const i::Address&>(data));
diff --git a/tests/test_api.rs b/tests/test_api.rs
index 6a16e7be..8e2f9d30 100644
--- a/tests/test_api.rs
+++ b/tests/test_api.rs
@@ -8501,6 +8501,30 @@ fn unbound_script_conversion() {
   }
 }
 
+#[test]
+fn ept_torture_test() {
+  let _setup_guard = setup::parallel_test();
+  let isolate = &mut v8::Isolate::new(Default::default());
+  // This should not OOM or crash when we run in a tight loop as the EPT should be subject
+  // to GC.
+  {
+    let scope = &mut v8::HandleScope::new(isolate);
+    let context = v8::Context::new(scope);
+    let scope = &mut v8::ContextScope::new(scope, context);
+    let source = v8::String::new(
+      scope,
+      r#"
+        for(let i = 0; i < 100_000; i++) new ArrayBuffer(1024 * 1024);
+        "OK";
+      "#,
+    )
+    .unwrap();
+    let script = v8::Script::compile(scope, source, None).unwrap();
+    let result = script.run(scope).unwrap();
+    assert_eq!(result.to_rust_string_lossy(scope), "OK");
+  }
+}
+
 #[test]
 fn run_with_rust_allocator() {
   use std::sync::Arc;
@@ -8586,6 +8610,25 @@ fn run_with_rust_allocator() {
   isolate.low_memory_notification();
   let count_loaded = count.load(Ordering::SeqCst);
   assert_eq!(count_loaded, 0);
+
+  // This should not OOM or crash when we run in a tight loop as the EPT should be subject
+  // to GC.
+  {
+    let scope = &mut v8::HandleScope::new(isolate);
+    let context = v8::Context::new(scope);
+    let scope = &mut v8::ContextScope::new(scope, context);
+    let source = v8::String::new(
+      scope,
+      r#"
+        for(let i = 0; i < 10_000; i++) new ArrayBuffer(10 * 1024 * 1024);
+        "OK";
+      "#,
+    )
+    .unwrap();
+    let script = v8::Script::compile(scope, source, None).unwrap();
+    let result = script.run(scope).unwrap();
+    assert_eq!(result.to_rust_string_lossy(scope), "OK");
+  }
 }
 
 // Same as heap_limits()