|
| 1 | +Source PR https://github.com/nodejs/node/pull/59623 |
| 2 | +commit 9d77c4191030576fd502faa04148b52fa6dbcb43 |
| 3 | +Author: Yaksh Bariya < [email protected]> |
| 4 | +Date: Mon Aug 25 14:19:59 2025 +0530 |
| 5 | + |
| 6 | + src: correctly report memory changes to V8 |
| 7 | + |
| 8 | + Call `V8::ExternalMemoryAccounter::Update` instead of |
| 9 | + `V8::ExternalMemoryAccounter::Increase` to report memory difference to |
| 10 | + V8 |
| 11 | + |
| 12 | + Calling `V8::ExternalMemoryAccounter::Increase` with a signed integer on |
| 13 | + 32-bit platforms causes instances where GC inside GC takes place leading |
| 14 | + to a crash in certain cases. |
| 15 | + |
| 16 | + During GC, native objects are destructed. In destructor for |
| 17 | + `CompressionStream` class used by zlib, memory release information is |
| 18 | + passed onto `V8::ExternalMemoryAccounter::Increase()` instead of |
| 19 | + `V8::ExternalMemoryAccounter::Decrease()` which triggers V8's memory |
| 20 | + limits, thus triggering GC inside GC which leads to crash. |
| 21 | + |
| 22 | + Bug initially introduced in commit |
| 23 | + 1d5d7b6eedb2274c9ad48b5f378598a10479e4a7 |
| 24 | + |
| 25 | + For full report see https://hackerone.com/reports/3302484 |
| 26 | + |
| 27 | +diff --git a/src/node_mem-inl.h b/src/node_mem-inl.h |
| 28 | +index 06871d031d3..70d28dd524b 100644 |
| 29 | +--- a/src/node_mem-inl.h |
| 30 | ++++ b/src/node_mem-inl.h |
| 31 | +@@ -59,7 +59,7 @@ void* NgLibMemoryManager<Class, T>::ReallocImpl(void* ptr, |
| 32 | + // Environment*/Isolate* parameter and call the V8 method transparently. |
| 33 | + const int64_t new_size = size - previous_size; |
| 34 | + manager->IncreaseAllocatedSize(new_size); |
| 35 | +- manager->env()->external_memory_accounter()->Increase( |
| 36 | ++ manager->env()->external_memory_accounter()->Update( |
| 37 | + manager->env()->isolate(), new_size); |
| 38 | + *reinterpret_cast<size_t*>(mem) = size; |
| 39 | + mem += sizeof(size_t); |
| 40 | +diff --git a/src/node_zlib.cc b/src/node_zlib.cc |
| 41 | +index c088c547539..b8617093bdf 100644 |
| 42 | +--- a/src/node_zlib.cc |
| 43 | ++++ b/src/node_zlib.cc |
| 44 | +@@ -644,7 +644,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { |
| 45 | + if (report == 0) return; |
| 46 | + CHECK_IMPLIES(report < 0, zlib_memory_ >= static_cast<size_t>(-report)); |
| 47 | + zlib_memory_ += report; |
| 48 | +- AsyncWrap::env()->external_memory_accounter()->Increase( |
| 49 | ++ AsyncWrap::env()->external_memory_accounter()->Update( |
| 50 | + AsyncWrap::env()->isolate(), report); |
| 51 | + } |
| 52 | + |
0 commit comments