Re: Native/javascript shared ArrayBuffer crash
Posted: Mon Mar 22, 2021 5:49 pm
Thanks for turning that around so fast! It's good and bad news. Good news is for a single frame with no navigating, the problem is fixed, yes. Bad news is that if I navigate in the frame (the frame is my document window, and when I open a file URL, it loads the new URL in the frame), I now get a debug check fail after the buffer is successfully released. I believe this is because the BackingStore is deleted _after_ the old frame context has been cleared and the new context has been created (the check fail occurs a couple seconds after the new page loads). The precise sequence of events is this:
1. Frame loads initial URL, allocates array buffer, everything fine. Can change the resolution causing new array buffer allocations, and later GC sweeps release those buffer without incident.
2. Open file dialog selects a new file URL. Calls CefFrame::LoadURL.
3. ~V8TrackArrayBuffer is called, which calls release buffer callback successfully, stack trace:
4. The new URL is loaded -- I can see it render, and I can see a new array buffer allocation. It renders fine for a couple seconds.
5. A GC sweep calls ~BackingStore, which tests the weakptr in the custom deleter, which does a DCHECK on a "valid sequence", which fails. Stack trace:
So, I don't understand the semantics here: should the BackingStore linger after the context has been released, and asynchronously deleted later? It seems like probably not. But I'm not sure why the BackingStore wouldn't be deleted as part of the context being released. Also it still seems a bit weird that the V8TrackArrayBuffer and the BackingStore both might be responsible for calling the release buffer callback. But it seems like the V8TrackArrayBuffer does it successfully now and the BackingStore's sort-of-ownership of the buffer isn't necessary. Over the weekend I tested stubbing out the custom deleter for the BackingStore (see diff below) and this worked fine (though obviously further muddying the ownership semantics).
1. Frame loads initial URL, allocates array buffer, everything fine. Can change the resolution causing new array buffer allocations, and later GC sweeps release those buffer without incident.
2. Open file dialog selects a new file URL. Calls CefFrame::LoadURL.
3. ~V8TrackArrayBuffer is called, which calls release buffer callback successfully, stack trace:
4. The new URL is loaded -- I can see it render, and I can see a new array buffer allocation. It renders fine for a couple seconds.
5. A GC sweep calls ~BackingStore, which tests the weakptr in the custom deleter, which does a DCHECK on a "valid sequence", which fails. Stack trace:
So, I don't understand the semantics here: should the BackingStore linger after the context has been released, and asynchronously deleted later? It seems like probably not. But I'm not sure why the BackingStore wouldn't be deleted as part of the context being released. Also it still seems a bit weird that the V8TrackArrayBuffer and the BackingStore both might be responsible for calling the release buffer callback. But it seems like the V8TrackArrayBuffer does it successfully now and the BackingStore's sort-of-ownership of the buffer isn't necessary. Over the weekend I tested stubbing out the custom deleter for the BackingStore (see diff below) and this worked fine (though obviously further muddying the ownership semantics).
- Code: Select all
--- chromium/src/v8/src/objects/backing-store.cc
+++ chromium/src/v8/src/objects/backing-store.cc
@@ -192,8 +192,10 @@ BackingStore::~BackingStore() {
DCHECK(free_on_destruct_);
TRACE_BS("BS:custom deleter bs=%p mem=%p (length=%zu, capacity=%zu)\n",
this, buffer_start_, byte_length(), byte_capacity_);
- type_specific_data_.deleter.callback(buffer_start_, byte_length_,
- type_specific_data_.deleter.data);
+ if (type_specific_data_.deleter.callback) {
+ type_specific_data_.deleter.callback(buffer_start_, byte_length_,
+ type_specific_data_.deleter.data);
+ }
Clear();
return;
}
--- chromium/src/cef/libcef/renderer/v8_impl.cc
+++ chromium/src/cef/libcef/renderer/v8_impl.cc
@@ -1416,15 +1416,15 @@ CefRefPtr<CefV8Value> CefV8Value::CreateArrayBuffer(
V8TrackArrayBuffer* tracker =
new V8TrackArrayBuffer(isolate, buffer, release_callback);
- auto deleter = [](void* data, size_t length, void* deleter_data) {
+/* auto deleter = [](void* data, size_t length, void* deleter_data) {
auto* tracker = reinterpret_cast<V8TrackArrayBuffer*>(deleter_data);
if (tracker) {
tracker->ReleaseBuffer();
}
- };
+ };*/
std::unique_ptr<v8::BackingStore> backing =
- v8::ArrayBuffer::NewBackingStore(buffer, length, deleter, tracker);
+ v8::ArrayBuffer::NewBackingStore(buffer, length, nullptr, nullptr);
v8::Local<v8::ArrayBuffer> ab =
v8::ArrayBuffer::New(isolate, std::move(backing));