Skip to content

Avoid crash in Chrome, issue 41487612 #16893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seanbudd merged 6 commits into from
Jul 29, 2024
Merged

Avoid crash in Chrome, issue 41487612 #16893

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aad11f1
Gecko_ia2 vbufBackend: ensure we release the document root accessible…
michaelDCurran Jul 22, 2024
1709917
Gecko vbufBackend: add assertion
michaelDCurran Jul 23, 2024
9a41e9f
Clarify comment
michaelDCurran Jul 23, 2024
0a151bc
Update what's new
michaelDCurran Jul 23, 2024
bd7b688
lint fixes
seanbudd Jul 28, 2024
13caf00
Update what's new
michaelDCurran Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,12 @@ void GeckoVBufBackend_t::renderThread_initialize() {
void GeckoVBufBackend_t::renderThread_terminate() {
unregisterWinEventHook(renderThread_winEventProcHook);
VBufBackend_t::renderThread_terminate();
// The backend holds a reference to the root accessible of the document.
// This must be specifically released here, in the UI thread where it was created.
// See https://issues.chromium.org/issues/41487612
if(this->rootDocAcc) {
Comment thread
seanbudd marked this conversation as resolved.
Resolved
this->rootDocAcc.Release();
}
}

void GeckoVBufBackend_t::render(VBufStorage_buffer_t* buffer, int docHandle, int ID, VBufStorage_controlFieldNode_t* oldNode) {
Expand All @@ -1502,6 +1508,18 @@ GeckoVBufBackend_t::GeckoVBufBackend_t(int docHandle, int ID): VBufBackend_t(doc
}

GeckoVBufBackend_t::~GeckoVBufBackend_t() {
// The backend holds a reference to the root accessible of the document.
// This must be specifically released in the UI thread where it was created.
// See https://issues.chromium.org/issues/41487612
// In most cases this will be released in renderThread_terminate.
// However in the unlikely case terminate can't run,
// We must detach and leak the COM pointer here,
Comment thread
seanbudd marked this conversation as resolved.
Resolved
// Otherwise it would be automatically deleted along with the backend which would cause a crash,
Comment thread
seanbudd marked this conversation as resolved.
Resolved
// as the COM object would be released from within an RPC worker thread.
nhAssert(!rootDocAcc);
if(this->rootDocAcc) {
Comment thread
seanbudd marked this conversation as resolved.
Resolved
this->rootDocAcc.Detach();
}
}

VBufBackend_t* GeckoVBufBackend_t_createInstance(int docHandle, int ID) {
Expand Down
1 change: 1 addition & 0 deletions user_docs/en/changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ A warning message will inform you if you try writing to a non-empty directory. (
* Playing NVDA sounds no longer fails on a mono audio device. (#16770, @jcsteh)
* NVDA will report addresses when arrowing through To/CC/BCC fields in outlook.com / Modern Outlook. (#16856)
* NVDA now handles add-on installation failures more gracefully. (#16704)
* No longer cause Google Chrome to crash when closing a document or exiting Chrome. (#16893)

@seanbudd seanbudd Jul 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this to the "web browser fixes" sub-list at the start of bug fixes


### Changes for Developers

Expand Down