Skip to content
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 on WebWorker destruction #35422

Merged
merged 1 commit into from Aug 29, 2022
Merged

fix: crash on WebWorker destruction #35422

merged 1 commit into from Aug 29, 2022

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Aug 24, 2022

Description of Change

Closes #35384.
Closes #35442.

Fixes a potential crash on WebWorker destruction. This was happening as a result of our custom UV_LOOP_INTERRUPT_ON_IO_CHANGE code added to libuv, as uv_loop_close results in a call to uv__io_stop, which then calls uv__loop_interrupt. uv__loop_interrupt didn't account for the case where the loop was no longer alive, and so would crash. This fixes that.

This PR also switches over from a direct call to uv_loop_close to Node's wrapper CheckedUvLoopClose, which calls uv_loop_close as expected but also aids in future debugging by logging all open handles should they exist.

Checklist

Release Notes

Notes: Fixed a potential crash on WebWorker destruction.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/19-x-y target/21-x-y PR should also be added to the "21-x-y" branch. labels Aug 24, 2022
@codebytere codebytere requested review from a team as code owners August 24, 2022 12:54
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 24, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 25, 2022
@deepak1556
Copy link
Member

uv__loop_interrupt didn't account for the case where the loop was no longer alive, and so would nullptr crash. This fixes that.

@codebytere which call ends up with nullptr ? The fix in this PR seems to more likely avoid the abort call instead if I am not mistaken.

@zcbenz
Copy link
Member

zcbenz commented Aug 29, 2022

It seems that it is abort crash instead of nullptr crash.

From the crash report:

Crashed Thread:        20  DedicatedWorker thread

Exception Type:        EXC_CRASH (SIGABRT)
Exception Codes:       0x0000000000000000, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Application Specific Information:
abort() called

@codebytere
Copy link
Member Author

That's correct. It is an abort crash, i miswrote it.

@codebytere
Copy link
Member Author

#
# Fatal error in ..\..\v8\src\builtins\profile-data-reader.cc, line 52
# Check failed: Can't read log file.
#
#

is a known flake cc @VerteDinde

@codebytere codebytere merged commit 1847581 into main Aug 29, 2022
@codebytere codebytere deleted the use-checked-close branch August 29, 2022 13:45
@release-clerk
Copy link

release-clerk bot commented Aug 29, 2022

Release Notes Persisted

Fixed a potential crash on WebWorker destruction.

@trop
Copy link
Contributor

trop bot commented Aug 29, 2022

I have automatically backported this PR to "19-x-y", please check out #35490

@trop
Copy link
Contributor

trop bot commented Aug 29, 2022

I have automatically backported this PR to "20-x-y", please check out #35491

@trop
Copy link
Contributor

trop bot commented Aug 29, 2022

I have automatically backported this PR to "21-x-y", please check out #35492

@trop trop bot added in-flight/20-x-y merged/21-x-y PR was merged to the "21-x-y" branch. and removed target/20-x-y target/21-x-y PR should also be added to the "21-x-y" branch. labels Aug 29, 2022
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/21-x-y PR was merged to the "21-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Upgrading to 19.0.13 break webpack hot reloading [Bug]: Mac crash on WebWorker destructor
3 participants