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.
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
fix: crash when invoking login callback synchronously #30068
Conversation
test failures are unrelated; merging. |
Release Notes Persisted
|
I was unable to backport this PR to "13-x-y" cleanly; |
I was unable to backport this PR to "12-x-y" cleanly; |
I have automatically backported this PR to "14-x-y", please check out #30090 |
Description of Change
Closes #30065.
This is because somehow the LoginHandler is deleted while calling Emit(), so
accessing the
auth_required_callback_
member variable after that is asegmentation fault.
It looks like calling
cb()
from JS ultimately ends up deleting theLoginHandler:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/storage_partition_impl.cc;l=523;drc=ded7717f5252a0c9d9de56bc22483b195d1801e0.
So this crashes:
electron/shell/browser/login_handler.cc
Line 73 in ccfde6c
this
has been deleted.We can work around that by taking a weak ptr before the
Emit
call and checking it afterwards, to guard against deletion. In that situation,auth_required_callback_
would have been null anyway, so there shouldn't be any behavior change.Checklist
npm test
passesRelease Notes
Notes: Fixed a crash when calling the
webContents.on('login')
callback synchronously.