10000 fix: process.exit crash in nativeWindowOpen by codebytere · Pull Request #30218 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: process.exit crash in nativeWindowOpen #30218

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

Jump to bottom
Merged
merged 3 commits into from
Jul 22, 2021
Merged

fix: process.exit crash in nativeWindowOpen #30218

merged 3 commits into from
Jul 22, 2021

Conversation

codebytere
Copy link
Member
@codebytere codebytere commented Jul 20, 2021 •
edited
Loading

Description of Change

Fixes #29969.

If nativeWindowOpen is enabled and we call process.exit() we perform a frame swap during window creation such that there's about a tick where Chromium is still working on the window at the same time we're destroying it via electron::WindowList::DestroyAllWindows().

This creates this specific race condition where the underlying ElectronNSWindow will have been freed when we call into NativeWindowMac. We can fix this by ensuring we do not call DidFirstVisuallyNonEmptyPaint() if the window is closed.

Tested with https://gist.github.com/c45a3d632e1740fad6d3d8bb351af654.

Checklist

Release Notes

Notes: Fixed a crash when process.exit is called and nativeWindowOpen is enabled.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/14-x-y labels Jul 20, 2021
@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Jul 20, 2021
@codebytere codebytere marked this pull request as ready for review July 21, 2021 10:02
@codebytere codebytere requested a review from nornagon July 21, 2021 10:04
@codebytere codebytere requested a review from jkleinsc July 21, 2021 16:52
ckerr
ckerr approved these changes Jul 21, 2021
View reviewed changes
Copy link
Member
@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

github really needs a :laughcry: emoji for times like this

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Jul 21, 2021
@codebytere
Copy link
Member Author

Failure:

webFrameMain module WebFrame.visibilityState should match window state

Is a known and unrelated flake. Merging.

@codebytere codebytere merged commit b24cfe1 into main Jul 22, 2021
@codebytere codebytere deleted the fix-native-window-exit-crash branch July 22, 2021 13:38
@release-clerk
Copy link
release-clerk bot commented Jul 22, 2021

Release Notes Persisted

Fixed a crash when process.exit is called and nativeWindowOpen is enabled.

@trop trop bot mentioned this pull request Jul 22, 2021
Merged
@trop
Copy link
Contributor
trop bot commented Jul 22, 2021

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

@jkleinsc
Copy link
Member

/trop run backport-to 15-x-y

@trop
Copy link
Contributor
trop bot commented Jul 22, 2021

The backport process for this PR has been manually initiated - sending your PR to 15-x-y!

@trop trop bot mentioned this pull request Jul 22, 2021
Merged
@trop
Copy link
Contributor
trop bot commented Jul 22, 2021

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

georgexu99 pushed a commit to georgexu99/electron that referenced this pull request Jul 29, 2021
@codebytere
fix: process.exit crash in nativeWindowOpen (electron#30218)
153af90
BlackHole1 pushed a commit to BlackHole1/electron that referenced this pull request Aug 30, 2021
@codebytere @BlackHole1
fix: process.exit crash in nativeWindowOpen (electron#30218)
b5dc8d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ckerr ckerr ckerr approved these changes

@MarshallOfSound MarshallOfSound MarshallOfSound approved these changes

@nornagon nornagon Awaiting requested review from nornagon

@jkleinsc jkleinsc Awaiting requested review from jkleinsc

Assignees
No one assigned
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Segfault when calling BrowserWindow::DidFirstVisuallyNonEmptyPaint
4 participants
@codebytere @jkleinsc @ckerr @MarshallOfSound
0