[API] Pass OOMDetails to OOMErrorCallback

This adds a new struct "OOMDetails" which is passed to the
OOMErrorCallback. It currently holds the "is_heap_oom" bool that was
also passed before, plus an optional "detail" string.
The struct can later be extended without having to change the signature
of the OOMErrorCallback. Removing fields will have to follow the
standard deprecation rules, but this is also easily possible without the
hassle for this initial change.

We modify the deprecated OOMErrorCallback definition and un-deprecate it,
which can be seen as removing a deprecated API and adding a new one in
one CL.

R=mlippautz@chromium.org, jkummerow@chromium.org

Bug: chromium:1323177
Change-Id: Ic4c2cb5856906ebd664626fe463d8e96cb99b0a5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3647827Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80565}

include/v8-callbacks.h

@@ -216,14 +216,17 @@ using AddHistogramSampleCallback = void (*)(void* histogram, int sample);
 
 using FatalErrorCallback = void (*)(const char* location, const char* message);
 
-using LegacyOOMErrorCallback = void (*)(const char* location, bool is_heap_oom);
-
-// TODO(chromium:1323177): Add a parameter for details, once this is deprecated
-// for at least one branch.
-using OOMErrorCallback V8_DEPRECATED(
-    "Use LegacyOOMErrorCallback; OOMErrorCallback will be changed "
-    "(https://crbug.com/1323177)") = void (*)(const char* location,
-                                              bool is_heap_oom);
+using LegacyOOMErrorCallback V8_DEPRECATE_SOON(
+    "Use OOMErrorCallback (https://crbug.com/1323177)") =
+    void (*)(const char* location, bool is_heap_oom);
+
+struct OOMDetails {
+  bool is_heap_oom = false;
+  const char* detail = nullptr;
+};
+
+using OOMErrorCallback = void (*)(const char* location,
+                                  const OOMDetails& details);
 
 using MessageCallback = void (*)(Local<Message> message, Local<Value> data);
 

include/v8-initialization.h

@@ -284,6 +284,9 @@ class V8_EXPORT V8 {
    * v8 has encountered a fatal failure to allocate memory and is about to
    * terminate.
    */
+  static void SetFatalMemoryErrorCallback(OOMErrorCallback callback);
+
+  V8_DEPRECATE_SOON("Use OOMErrorCallback (https://crbug.com/1323177)")
   static void SetFatalMemoryErrorCallback(LegacyOOMErrorCallback callback);
 
   /**

include/v8-isolate.h

@@ -286,12 +286,11 @@ class V8_EXPORT Isolate {
      * Callbacks to invoke in case of fatal or OOM errors.
      */
     FatalErrorCallback fatal_error_callback = nullptr;
-    LegacyOOMErrorCallback legacy_oom_error_callback = nullptr;
-    V8_DEPRECATED(
-        "Use legacy_oom_error_callback; OOMErrorCallback will be changed soon "
-        "(https://crbug.com/1323177)")
     OOMErrorCallback oom_error_callback = nullptr;
 
+    V8_DEPRECATE_SOON("Use oom_error_callback (https://crbug.com/1323177)")
+    LegacyOOMErrorCallback legacy_oom_error_callback = nullptr;
+
     /**
      * The following parameter is experimental and may change significantly.
      * This is currently for internal testing.
@@ -1478,9 +1477,13 @@ class V8_EXPORT Isolate {
   /** Set the callback to invoke in case of fatal errors. */
   void SetFatalErrorHandler(FatalErrorCallback that);
 
-  /** Set the callback to invoke in case of OOM errors. */
+  /** Set the callback to invoke in case of OOM errors (deprecated). */
+  V8_DEPRECATE_SOON("Use OOMErrorCallback (https://crbug.com/1323177)")
   void SetOOMErrorHandler(LegacyOOMErrorCallback that);
 
+  /** Set the callback to invoke in case of OOM errors. */
+  void SetOOMErrorHandler(OOMErrorCallback that);
+
   /**
    * Add a callback to invoke in case the heap size is close to the heap limit.
    * If multiple callbacks are added, only the most recently added callback is

src/api/api.cc

@@ -167,9 +167,14 @@
 
 namespace v8 {
 
-// TODO(chromium:1323177): Add a separate global for OOMErrorCallback once the
-// types diverge.
-static LegacyOOMErrorCallback g_oom_error_callback = nullptr;
+// Redefine LegacyOOMErrorCallback here for internal usage. We still need to
+// support it but it is deprecated so would trigger warnings.
+// TODO(chromium:1323177): Remove this.
+using DeprecatedLegacyOOMErrorCallback = void (*)(const char* location,
+                                                  bool is_heap_oom);
+
+static DeprecatedLegacyOOMErrorCallback g_legacy_oom_error_callback = nullptr;
+static OOMErrorCallback g_oom_error_callback = nullptr;
 
 static ScriptOrigin GetScriptOriginForScript(i::Isolate* i_isolate,
                                              i::Handle<i::Script> script) {
@@ -207,7 +212,7 @@ Local<PrimitiveArray> ScriptOrigin::HostDefinedOptions() const {
 // When V8 cannot allocate memory FatalProcessOutOfMemory is called. The default
 // OOM error handler is called and execution is stopped.
 void i::V8::FatalProcessOutOfMemory(i::Isolate* i_isolate, const char* location,
-                                    bool is_heap_oom) {
+                                    const OOMDetails& details) {
   char last_few_messages[Heap::kTraceRingBufferSize + 1];
   char js_stacktrace[Heap::kStacktraceBufferSize + 1];
   i::HeapStats heap_stats;
@@ -225,7 +230,10 @@ void i::V8::FatalProcessOutOfMemory(i::Isolate* i_isolate, const char* location,
     memset(&heap_stats, 0xBADC0DE, sizeof(heap_stats));
     // Give the embedder a chance to handle the condition. If it doesn't,
     // just crash.
-    if (g_oom_error_callback) g_oom_error_callback(location, is_heap_oom);
+    if (g_oom_error_callback) g_oom_error_callback(location, details);
+    if (g_legacy_oom_error_callback) {
+      g_legacy_oom_error_callback(location, details.is_heap_oom);
+    }
     FATAL("Fatal process out of memory: %s", location);
     UNREACHABLE();
   }
@@ -299,8 +307,11 @@ void i::V8::FatalProcessOutOfMemory(i::Isolate* i_isolate, const char* location,
       base::OS::PrintError("\n<--- JS stacktrace --->\n%s\n", js_stacktrace);
     }
   }
-  Utils::ReportOOMFailure(i_isolate, location, is_heap_oom);
-  if (g_oom_error_callback) g_oom_error_callback(location, is_heap_oom);
+  Utils::ReportOOMFailure(i_isolate, location, details);
+  if (g_oom_error_callback) g_oom_error_callback(location, details);
+  if (g_legacy_oom_error_callback) {
+    g_legacy_oom_error_callback(location, details.is_heap_oom);
+  }
   // If the fatal error handler returns, we stop execution.
   FATAL("API fatal error handler returned after process out of memory");
 }
@@ -322,15 +333,19 @@ void Utils::ReportApiFailure(const char* location, const char* message) {
 }
 
 void Utils::ReportOOMFailure(i::Isolate* i_isolate, const char* location,
-                             bool is_heap_oom) {
-  LegacyOOMErrorCallback oom_callback = i_isolate->oom_behavior();
-  if (oom_callback == nullptr) {
+                             const OOMDetails& details) {
+  if (auto oom_callback = i_isolate->oom_behavior()) {
+    oom_callback(location, details);
+  } else if (auto legacy_oom_callback = i_isolate->legacy_oom_behavior()) {
+    legacy_oom_callback(location, details.is_heap_oom);
+  } else {
     // TODO(wfh): Remove this fallback once Blink is setting OOM handler. See
     // crbug.com/614440.
     FatalErrorCallback fatal_callback = i_isolate->exception_behavior();
     if (fatal_callback == nullptr) {
       base::OS::PrintError("\n#\n# Fatal %s OOM in %s\n#\n\n",
-                           is_heap_oom ? "javascript" : "process", location);
+                           details.is_heap_oom ? "javascript" : "process",
+                           location);
 #ifdef V8_FUZZILLI
       exit(0);
 #else
@@ -338,12 +353,10 @@ void Utils::ReportOOMFailure(i::Isolate* i_isolate, const char* location,
 #endif  // V8_FUZZILLI
     } else {
       fatal_callback(location,
-                     is_heap_oom
+                     details.is_heap_oom
                          ? "Allocation failed - JavaScript heap out of memory"
                          : "Allocation failed - process out of memory");
     }
-  } else {
-    oom_callback(location, is_heap_oom);
   }
   i_isolate->SignalFatalError();
 }
@@ -6129,10 +6142,15 @@ void V8::SetUnhandledExceptionCallback(
 #endif  // V8_OS_WIN
 
 void v8::V8::SetFatalMemoryErrorCallback(
-    v8::LegacyOOMErrorCallback oom_error_callback) {
+    v8::OOMErrorCallback oom_error_callback) {
   g_oom_error_callback = oom_error_callback;
 }
 
+void v8::V8::SetFatalMemoryErrorCallback(
+    v8::LegacyOOMErrorCallback legacy_oom_error_callback) {
+  g_legacy_oom_error_callback = legacy_oom_error_callback;
+}
+
 void v8::V8::SetEntropySource(EntropySource entropy_source) {
   base::RandomNumberGenerator::SetEntropySource(entropy_source);
 }
@@ -9393,7 +9411,9 @@ size_t Isolate::CopyCodePages(size_t capacity, MemoryRange* code_pages_out) {
   }
 
 CALLBACK_SETTER(FatalErrorHandler, FatalErrorCallback, exception_behavior)
-CALLBACK_SETTER(OOMErrorHandler, LegacyOOMErrorCallback, oom_behavior)
+CALLBACK_SETTER(OOMErrorHandler, OOMErrorCallback, oom_behavior)
+CALLBACK_SETTER(OOMErrorHandler, DeprecatedLegacyOOMErrorCallback,
+                legacy_oom_behavior)
 CALLBACK_SETTER(ModifyCodeGenerationFromStringsCallback,
                 ModifyCodeGenerationFromStringsCallback2,
                 modify_code_gen_callback2)

src/api/api.h

@@ -156,7 +156,7 @@ class Utils {
     return condition;
   }
   static void ReportOOMFailure(v8::internal::Isolate* isolate,
-                               const char* location, bool is_heap_oom);
+                               const char* location, const OOMDetails& details);
 
   static inline Local<debug::AccessorPair> ToLocal(
       v8::internal::Handle<v8::internal::AccessorPair> obj);

src/execution/isolate.h

@@ -476,10 +476,17 @@ V8_EXPORT_PRIVATE void FreeCurrentEmbeddedBlob();
 
 using DebugObjectCache = std::vector<Handle<HeapObject>>;
 
+// Redefine LegacyOOMErrorCallback here for internal usage. We still need to
+// support it but it is deprecated so would trigger warnings.
+// TODO(chromium:1323177): Remove this.
+using DeprecatedLegacyOOMErrorCallback = void (*)(const char* location,
+                                                  bool is_heap_oom);
+
 #define ISOLATE_INIT_LIST(V)                                                  \
   /* Assembler state. */                                                      \
   V(FatalErrorCallback, exception_behavior, nullptr)                          \
-  V(LegacyOOMErrorCallback, oom_behavior, nullptr)                            \
+  V(DeprecatedLegacyOOMErrorCallback, legacy_oom_behavior, nullptr)           \
+  V(OOMErrorCallback, oom_behavior, nullptr)                                  \
   V(LogEventCallback, event_logger, nullptr)                                  \
   V(AllowCodeGenerationFromStringsCallback, allow_code_gen_callback, nullptr) \
   V(ModifyCodeGenerationFromStringsCallback, modify_code_gen_callback,        \

src/heap/heap-allocator.cc

@@ -127,8 +127,8 @@ AllocationResult HeapAllocator::AllocateRawWithRetryOrFailSlowPath(
     return result;
   }
 
-  v8::internal::V8::FatalProcessOutOfMemory(heap_->isolate(),
-                                            "CALL_AND_RETRY_LAST", true);
+  V8::FatalProcessOutOfMemory(heap_->isolate(), "CALL_AND_RETRY_LAST",
+                              V8::kHeapOOM);
 }
 
 #ifdef DEBUG

src/heap/heap.cc

@@ -6363,7 +6363,7 @@ void Heap::CompactRetainedMaps(WeakArrayList retained_maps) {
 }
 
 void Heap::FatalProcessOutOfMemory(const char* location) {
-  v8::internal::V8::FatalProcessOutOfMemory(isolate(), location, true);
+  V8::FatalProcessOutOfMemory(isolate(), location, V8::kHeapOOM);
 }
 
 #ifdef DEBUG

src/heap/promote-young-generation.cc

@@ -49,7 +49,8 @@ void PromoteYoungGenerationGC::EvacuateYoungGeneration() {
   // Reset new space.
   semi_space_new_space->EvacuatePrologue();
   if (!semi_space_new_space->Rebalance()) {
-    V8::FatalProcessOutOfMemory(heap_->isolate(), "NewSpace::Rebalance", true);
+    V8::FatalProcessOutOfMemory(heap_->isolate(), "NewSpace::Rebalance",
+                                V8::kHeapOOM);
   }
   semi_space_new_space->set_age_mark(semi_space_new_space->top());
 

src/init/v8.cc

@@ -38,7 +38,10 @@
 namespace v8 {
 namespace internal {
 
+// static
 v8::Platform* V8::platform_ = nullptr;
+const OOMDetails V8::kNoOOMDetails{false, nullptr};
+const OOMDetails V8::kHeapOOM{true, nullptr};
 
 namespace {
 enum class V8StartupState {

src/init/v8.h

@@ -9,6 +9,7 @@
 
 namespace v8 {
 
+struct OOMDetails;
 class Platform;
 class StartupData;
 
@@ -27,7 +28,13 @@ class V8 : public AllStatic {
   // IMPORTANT: Update the Google-internal crash processer if this signature
   // changes to be able to extract detailed v8::internal::HeapStats on OOM.
   [[noreturn]] V8_EXPORT_PRIVATE static void FatalProcessOutOfMemory(
-      Isolate* isolate, const char* location, bool is_heap_oom = false);
+      Isolate* isolate, const char* location,
+      const OOMDetails& details = kNoOOMDetails);
+
+  // Constants to be used for V8::FatalProcessOutOfMemory. They avoid having
+  // to include v8-callbacks.h in all callers.
+  V8_EXPORT_PRIVATE static const OOMDetails kNoOOMDetails;
+  V8_EXPORT_PRIVATE static const OOMDetails kHeapOOM;
 
 #ifdef V8_ENABLE_SANDBOX
   static bool InitializeSandbox();

test/cctest/heap/test-heap.cc

@@ -6819,7 +6819,7 @@ UNINITIALIZED_TEST(ReinitializeStringHashSeed) {
 const int kHeapLimit = 100 * MB;
 Isolate* oom_isolate = nullptr;
 
-void OOMCallback(const char* location, bool is_heap_oom) {
+void OOMCallback(const char* location, const OOMDetails&) {
   Heap* heap = oom_isolate->heap();
   size_t kSlack = heap->new_space() ? heap->MaxSemiSpaceSize() : 0;
   CHECK_LE(heap->OldGenerationCapacity(), kHeapLimit + kSlack);