Skip to content

Commit fc9444f

Browse files
committed
inspector: avoid v8::String allocation in fromStringView
fromStringView() was round-tripping the inspector StringView through v8::String::NewFromOneByte / NewFromTwoByte just to use Utf8Value for the UTF-16 to UTF-8 conversion. That allocation is into V8's managed heap, which is forbidden during certain V8 internal phases -- most notably weak callbacks during garbage collection. The concrete failure path that motivated this fix: v8::internal::HeapAllocator::AllocateRaw (debug check) v8::String::NewFromOneByte fromStringView v8_inspector__Channel__IMPL::sendResponse v8_crdtp::DomainDispatcher::sendResponse v8_inspector::EvaluateCallback::sendFailure v8_inspector::PromiseHandlerTracker::sendFailure v8_inspector::PromiseHandlerTracker::discard v8::internal::GlobalHandles::InvokeFirstPassWeakCallbacks v8::internal::Heap::PerformGarbageCollection V8 invokes inspector callbacks (specifically PromiseHandlerTracker::discard, which sends a failure response for a stale Runtime.evaluate) from inside GC's first-pass weak-callbacks phase. Sending the response goes through this helper; the v8::String::NewFromOneByte allocation then trips AllowHeapAllocation::IsAllowed() and aborts the process in debug builds (in release builds it's undefined behavior, since allocations during weak callbacks can corrupt the heap walk). The conversion doesn't actually need V8 at all -- it's just bytes in, UTF-8 bytes out. Use the same approach allocString() already uses (the call site immediately below this one in the file): direct construction of std::string for the 8-bit case, v8_inspector::UTF16ToUTF8 for the 16-bit case. v8_inspector::UTF16ToUTF8 is a pure host-side helper that doesn't touch the V8 heap. The isolate parameter is kept for API compatibility but is no longer referenced. Reproduces with lightpanda-io/browser#2407 (puppeteer-core repro against any iframe-heavy page that runs Web Workers; the inspector keeps a Runtime.evaluate callback alive past the worker / page lifetime, GC eventually fires the discard, and the v8::String allocation aborts the process). With this change, the same 25-iteration back-to-back stress test runs to completion without the V8 fatal.
1 parent 0eef7f5 commit fc9444f

1 file changed

Lines changed: 29 additions & 8 deletions

File tree

src/binding.cpp

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2440,14 +2440,35 @@ static inline v8_inspector::StringView toStringView(const std::string &str) {
24402440
}
24412441

24422442
static inline std::string fromStringView(v8::Isolate* isolate, const v8_inspector::StringView stringView) {
2443-
int length = static_cast<int>(stringView.length());
2444-
v8::Local<v8::String> message = (
2445-
stringView.is8Bit()
2446-
? v8::String::NewFromOneByte(isolate, stringView.characters8(), v8::NewStringType::kNormal, length)
2447-
: v8::String::NewFromTwoByte(isolate, stringView.characters16(), v8::NewStringType::kNormal, length)
2448-
).ToLocalChecked();
2449-
v8::String::Utf8Value result(isolate, message);
2450-
return *result;
2443+
// Convert the inspector StringView to UTF-8 *without* allocating a
2444+
// v8::String. The previous implementation round-tripped through
2445+
// v8::String::NewFromOneByte / NewFromTwoByte (just to use Utf8Value
2446+
// for the conversion), which calls into V8's heap allocator. That is
2447+
// forbidden during certain V8 internal phases -- in particular,
2448+
// weak callbacks during garbage collection, where V8's
2449+
// PromiseHandlerTracker can invoke EvaluateCallback::sendFailure,
2450+
// which lands in Channel::sendResponse, which calls this helper.
2451+
// Allocating in that context trips AllowHeapAllocation::IsAllowed()
2452+
// and aborts the process in debug builds (in release builds the
2453+
// allocation is undefined behavior).
2454+
//
2455+
// The 8-bit case can be a direct memcpy; the 16-bit case uses
2456+
// v8_inspector::UTF16ToUTF8 (a pure host-side helper that doesn't
2457+
// touch the V8 heap) -- the same path allocString() uses.
2458+
//
2459+
// The `isolate` parameter is preserved for API compatibility but is
2460+
// no longer needed.
2461+
(void)isolate;
2462+
if (stringView.is8Bit()) {
2463+
return std::string(
2464+
reinterpret_cast<const char*>(stringView.characters8()),
2465+
stringView.length()
2466+
);
2467+
}
2468+
return v8_inspector::UTF16ToUTF8(
2469+
reinterpret_cast<const char16_t*>(stringView.characters16()),
2470+
stringView.length()
2471+
);
24512472
}
24522473

24532474
/// Allocates a string as utf8 on the allocator without \0 terminator, for use in Zig.

0 commit comments

Comments
 (0)