deps: fix V8 race condition for AIX

abmusse · targos · commit b3889d5a306a · 2026-04-24T07:21:00.000+02:00
aix: simplify OS::DecommitPages implementation Replace complex mmap/munmap retry logic with mprotect + madvise approach. This fixes a race condition that was causing test failures in Node.js. Node.js stress test was run with this fix and testing shows 0 failures out of 1000 runs of wpt/test-wasm-jsapi with this patch compared to 224 failures without it. Refs: #62647 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7780464
diff --git a/common.gypi b/common.gypi
@@ -40,7 +40,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.16',
+    'v8_embedder_string': '-node.17',
 
     ##### V8 defaults for Node.js #####
 
diff --git a/deps/v8/src/base/platform/platform-aix.cc b/deps/v8/src/base/platform/platform-aix.cc
@@ -171,34 +171,26 @@ bool OS::DecommitPages(void* address, size_t size) {
   // with MAP_FIXED will fail and return -1 unless the application has requested
   // SPEC1170 compliant behaviour:
   // https://www.ibm.com/docs/en/aix/7.3?topic=m-mmap-mmap64-subroutine
-  // Therefore in case if failure we need to unmap the address before trying to
-  // map it again. The downside is another thread could place another mapping at
-  // the same address after the munmap but before the mmap, therefore a CHECK is
-  // also added to assure the address is mapped successfully. Refer to the
-  // comments under https://crrev.com/c/3010195 for more details.
-#define MMAP() \
-  mmap(address, size, PROT_NONE, MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE, -1, 0)
+  // As a workaround we use `mprotect` to make the page inaccessible and
+  // `madvise` to hint the OS to release the memory.
+  //
+  // NOTE: On AIX, madvise() is a no-op and does not release physical
+  // memory. The pages remain protected but the physical memory may only be
+  // reclaimed by the OS under memory pressure. This trade-off is acceptable
+  // because:
+  // 1. It fixes critical race conditions (Refs:
+  // https://github.com/nodejs/node/issues/62647)
+  // 2. AIX's disclaim64() requires writable pages, incompatible with PROT_NONE
+  // protection
+  // 3. The alternative munmap/mmap approach causes "Check failed: ptr ==
+  // address" errors
+  // 4. Pages are inaccessible, preventing memory corruption
+
   DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
   DCHECK_EQ(0, size % CommitPageSize());
-  void* ptr;
-  // Try without mapping first.
-  ptr = MMAP();
-  if (ptr != address) {
-    DCHECK_EQ(ptr, MAP_FAILED);
-    // Returns 0 when successful.
-    if (munmap(address, size)) {
-      return false;
-    }
-    // Try again after unmap.
-    ptr = MMAP();
-    // If this check fails it's most likely due to a racing condition where
-    // another thread has mapped the same address right before we do.
-    // Since this could cause hard-to-debug issues, potentially with security
-    // impact, and we can't recover from this, the best we can do is abort the
-    // process.
-    CHECK_EQ(ptr, address);
-  }
-#undef MMAP
+  if (mprotect(address, size, PROT_NONE) != 0) return false;
+  if (madvise(static_cast<caddr_t>(address), size, MADV_DONTNEED) != 0)
+    return false;
   return true;
 }
 
