deps: V8: cherry-pick 3176bfd447a9

common.gypi

@@ -36,7 +36,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 #####
 

deps/v8/src/api/api.cc

@@ -10716,7 +10716,8 @@ static i::HeapSnapshot* ToInternal(const HeapSnapshot* snapshot) {
 
 void HeapSnapshot::Delete() {
   i::Isolate* isolate = ToInternal(this)->profiler()->isolate();
-  if (isolate->heap_profiler()->GetSnapshotsCount() > 1) {
+  if (isolate->heap_profiler()->GetSnapshotsCount() > 1 ||
+      isolate->heap_profiler()->IsTakingSnapshot()) {
     ToInternal(this)->Delete();
   } else {
     // If this is the last snapshot, clean up all accessory data as well.

deps/v8/src/profiler/heap-profiler.cc

@@ -18,7 +18,8 @@ namespace internal {
 HeapProfiler::HeapProfiler(Heap* heap)
     : ids_(new HeapObjectsMap(heap)),
       names_(new StringsStorage()),
-      is_tracking_object_moves_(false) {}
+      is_tracking_object_moves_(false),
+      is_taking_snapshot_(false) {}
 
 HeapProfiler::~HeapProfiler() = default;
 
@@ -28,7 +29,8 @@ void HeapProfiler::DeleteAllSnapshots() {
 }
 
 void HeapProfiler::MaybeClearStringsStorage() {
-  if (snapshots_.empty() && !sampling_heap_profiler_ && !allocation_tracker_) {
+  if (snapshots_.empty() && !sampling_heap_profiler_ && !allocation_tracker_ &&
+      !is_taking_snapshot_) {
     names_.reset(new StringsStorage());
   }
 }
@@ -66,6 +68,7 @@ HeapSnapshot* HeapProfiler::TakeSnapshot(
     v8::ActivityControl* control,
     v8::HeapProfiler::ObjectNameResolver* resolver,
     bool treat_global_objects_as_roots) {
+  is_taking_snapshot_ = true;
   HeapSnapshot* result = new HeapSnapshot(this, treat_global_objects_as_roots);
   {
     HeapSnapshotGenerator generator(result, control, resolver, heap());
@@ -78,6 +81,7 @@ HeapSnapshot* HeapProfiler::TakeSnapshot(
   }
   ids_->RemoveDeadEntries();
   is_tracking_object_moves_ = true;
+  is_taking_snapshot_ = false;
 
   heap()->isolate()->debug()->feature_tracker()->Track(
       DebugFeatureTracker::kHeapSnapshot);
@@ -138,10 +142,12 @@ void HeapProfiler::StopHeapObjectsTracking() {
   }
 }
 
-int HeapProfiler::GetSnapshotsCount() {
+int HeapProfiler::GetSnapshotsCount() const {
   return static_cast<int>(snapshots_.size());
 }
 
+bool HeapProfiler::IsTakingSnapshot() const { return is_taking_snapshot_; }
+
 HeapSnapshot* HeapProfiler::GetSnapshot(int index) {
   return snapshots_.at(index).get();
 }

deps/v8/src/profiler/heap-profiler.h

@@ -49,7 +49,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
 
   SnapshotObjectId PushHeapObjectsStats(OutputStream* stream,
                                         int64_t* timestamp_us);
-  int GetSnapshotsCount();
+  int GetSnapshotsCount() const;
+  bool IsTakingSnapshot() const;
   HeapSnapshot* GetSnapshot(int index);
   SnapshotObjectId GetSnapshotObjectId(Handle<Object> obj);
   SnapshotObjectId GetSnapshotObjectId(NativeObject obj);
@@ -93,6 +94,7 @@ class HeapProfiler : public HeapObjectAllocationTracker {
   std::unique_ptr<StringsStorage> names_;
   std::unique_ptr<AllocationTracker> allocation_tracker_;
   bool is_tracking_object_moves_;
+  bool is_taking_snapshot_;
   base::Mutex profiler_mutex_;
   std::unique_ptr<SamplingHeapProfiler> sampling_heap_profiler_;
   std::vector<std::pair<v8::HeapProfiler::BuildEmbedderGraphCallback, void*>>

deps/v8/test/cctest/test-heap-profiler.cc

@@ -4126,3 +4126,40 @@ TEST(Bug8373_2) {
 
   heap_profiler->StopTrackingHeapObjects();
 }
+
+TEST(HeapSnapshotDeleteDuringTakeSnapshot) {
+  // Check that a heap snapshot can be deleted during GC while another one
+  // is being taken.
+
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+  int gc_calls = 0;
+  v8::Global<v8::Object> handle;
+
+  {
+    struct WeakData {
+      const v8::HeapSnapshot* snapshot;
+      int* gc_calls;
+      v8::Global<v8::Object>* handle;
+    };
+    WeakData* data =
+        new WeakData{heap_profiler->TakeHeapSnapshot(), &gc_calls, &handle};
+
+    v8::HandleScope scope(env->GetIsolate());
+    handle.Reset(env->GetIsolate(), v8::Object::New(env->GetIsolate()));
+    handle.SetWeak(
+        data,
+        [](const v8::WeakCallbackInfo<WeakData>& data) {
+          std::unique_ptr<WeakData> weakdata{data.GetParameter()};
+          const_cast<v8::HeapSnapshot*>(weakdata->snapshot)->Delete();
+          ++*weakdata->gc_calls;
+          weakdata->handle->Reset();
+        },
+        v8::WeakCallbackType::kParameter);
+  }
+  CHECK_EQ(gc_calls, 0);
+
+  CHECK(ValidateSnapshot(heap_profiler->TakeHeapSnapshot()));
+  CHECK_EQ(gc_calls, 1);
+}

test/parallel/test-v8-getheapsnapshot-twice.js

@@ -0,0 +1,9 @@
+'use strict';
+require('../common');
+const v8 = require('v8');
+
+// Regression test for https://github.com/nodejs/node/issues/35559
+// It is important that the return value of the first call is not used, i.e.
+// that the first snapshot is GC-able while the second one is being created.
+v8.getHeapSnapshot();
+v8.getHeapSnapshot();