-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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: do not destroy thread in UI thread #23495
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
1a8e739
to
c2e80a0
Compare
57242da
to
ba3a3f4
Compare
ba3a3f4
to
22a25ed
Compare
No Release Notes |
I was unable to backport this PR to "8-x-y" cleanly; |
I was unable to backport this PR to "7-2-x" cleanly; |
I have automatically backported this PR to "9-x-y", please check out #23535 |
Description of Change
Destroying a thread implies joining it, and it should not be done in UI thread, otherwise it would cause assertions when DCHECK is enabled:
The dialog module on Windows uses this behavior and crashes when used with DCHECK enabled.
This PR refactors the code to use ThreadPool to manage the threads to avoid allocating and destroying threads in UI thread, which follows the pattern used by
BaseShellDialogImpl
in Chromium. (We can not useBaseShellDialogImpl
directly because it implies modal dialog with a parent.)Checklist
npm test
passesRelease Notes
Notes: none