Skip to content

fix: add uv_loop_close when object release to fix crash - #35322

Merged
jkleinsc merged 1 commit into
electron:mainfrom
zuohuiyang:fix_call_lib_uv_crash
Aug 15, 2022
Merged

fix: add uv_loop_close when object release to fix crash#35322
jkleinsc merged 1 commit into
electron:mainfrom
zuohuiyang:fix_call_lib_uv_crash

Conversation

@zuohuiyang

@zuohuiyang zuohuiyang commented Aug 13, 2022 •
edited by jkleinsc
Loading

Copy link
Copy Markdown
Contributor

Description of Change

stop_and_close_uv_loop add call uv_loop_close.
In the destructor of NodeBindings, stop_and_close_uv_loop is called, but stop_and_close_uv_loop does not call uv_loop_close. After NodeBindings are destructed, libuv still holds the member objects of NodeBindings. When power resume, libuv enumerates all loops, and will accesses a pointer that has been released.
as issue #32926

About npm test in checklist. Before I modified the code, 15 cases could not pass on my computer. After I modified, the failed cases did not increase, so there should be no problem with npm test.

Resolves #32926

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes

Release Notes

Notes: Fixed crash in renderer when resuming system from sleep

@welcome

welcome Bot commented Aug 13, 2022

Copy link
Copy Markdown

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Aug 13, 2022
@zcbenz zcbenz 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 15, 2022
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Aug 15, 2022
@jkleinsc
jkleinsc merged commit cbc1ee5 into electron:main Aug 15, 2022
@welcome

welcome Bot commented Aug 15, 2022

Copy link
Copy Markdown

Congrats on merging your first pull request! 🎉🎉🎉

@release-clerk

release-clerk Bot commented Aug 15, 2022

Copy link
Copy Markdown

Release Notes Persisted

Fixed crash in renderer when resuming system from sleep

@trop trop Bot mentioned this pull request Aug 15, 2022
Merged
@trop

trop Bot commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

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

@trop trop Bot added in-flight/21-x-y and removed target/21-x-y PR should also be added to the "21-x-y" branch. labels Aug 15, 2022
This was referenced Aug 15, 2022
Merged
Merged
@trop

trop Bot commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

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

@trop

trop Bot commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

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

@miniak

miniak commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

/trop run backport-to 18-x-y

@trop

trop Bot commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

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

@trop trop Bot mentioned this pull request Aug 15, 2022
Merged
@trop

trop Bot commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

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

@trop trop Bot added in-flight/18-x-y merged/21-x-y PR was merged to the "21-x-y" branch. and removed in-flight/18-x-y labels Aug 15, 2022
@DanielMcAssey

DanielMcAssey commented Aug 19, 2022

Copy link
Copy Markdown

This seems to have broken in Mac with WebWorkers destructing. Will look at posting dump and repro

@DanielMcAssey DanielMcAssey mentioned this pull request Aug 19, 2022
Closed
3 tasks
@srubin srubin mentioned this pull request Aug 25, 2022
Closed
3 tasks
@dohard-ma dohard-ma mentioned this pull request Aug 31, 2022
Merged
@ghost ghost mentioned this pull request Oct 27, 2022
Open
schetle pushed a commit to schetle/electron that referenced this pull request Nov 3, 2022
@zuohuiyang yangzuohui @schetle
fix: add uv_loop_close when object release to fix crash (electron#35322)
21b4863 
Co-authored-by: yangzuohui <yangzuohui@bytedance.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
@zuohuiyang yangzuohui @khalwa
fix: add uv_loop_close when object release to fix crash (electron#35322)
01c424c 
Co-authored-by: yangzuohui <yangzuohui@bytedance.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes
@deepak1556 deepak1556 deepak1556 approved these changes
+1 more reviewer
@zcbenz zcbenz zcbenz approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

merged/21-x-y PR was merged to the "21-x-y" branch. semver/patch backwards-compatible bug fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Renderer process crashes on power resume

6 participants

@zuohuiyang @miniak @DanielMcAssey @jkleinsc @zcbenz @deepak1556