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: make StreamSubscriber ref counted #17221

Merged
merged 1 commit into from Mar 7, 2019
Merged

fix: make StreamSubscriber ref counted #17221

merged 1 commit into from Mar 7, 2019

Conversation

deepak1556
Copy link
Member

@deepak1556 deepak1556 commented Mar 4, 2019

Description of Change

It is owned by URLRequestStreamJob on the IO thread once request starts, but if the ownership was abondoned while transfering it to IO thread which is possible when a request is aborted, then we need to make sure its destroyed on the right thread.

Fixes #16881

Checklist

Release Notes

Notes: Fix app freeze when using custom stream protocol

It is owned by URLRequestStreamJob on the IO thread once request starts,
but if the ownership was abondoned while transfering it to IO thread
which is possible when a request is aborted, then we need to make sure
its destroyed on the right thread to avoid lock in v8.
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 4, 2019
@deepak1556
Copy link
Member Author

/cc @pfrazee

@pfrazee
Copy link
Contributor

pfrazee commented Mar 4, 2019

Will test this shortly, I'm just fighting with the electron build process

@pfrazee
Copy link
Contributor

pfrazee commented Mar 4, 2019

Just tested this against https://github.com/pfrazee/electron-bug-webview-freeze and confirmed the fix works 👍

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Mar 5, 2019
@deepak1556 deepak1556 merged commit d4d6b98 into master Mar 7, 2019
@release-clerk
Copy link

release-clerk bot commented Mar 7, 2019

Release Notes Persisted

Fix app freeze when using custom stream protocol

@deepak1556 deepak1556 deleted the stream_crash branch March 7, 2019 15:20
@trop
Copy link
Contributor

trop bot commented Mar 7, 2019

I was unable to backport this PR to "4-0-x" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Mar 7, 2019

I have automatically backported this PR to "5-0-x", please check out #17264

@trop
Copy link
Contributor

trop bot commented Mar 7, 2019

A maintainer has manually backported this PR to "4-0-x", please check out #17267

@sofianguy sofianguy added this to Fixed in 5.0.0-beta.6 in 5.0.x Mar 20, 2019
@trop
Copy link
Contributor

trop bot commented Mar 29, 2019

A maintainer has manually backported this PR to "4-1-x", please check out #17267

kiku-jw pushed a commit to kiku-jw/electron that referenced this pull request May 16, 2019
It is owned by URLRequestStreamJob on the IO thread once request starts,
but if the ownership was abondoned while transfering it to IO thread
which is possible when a request is aborted, then we need to make sure
its destroyed on the right thread to avoid lock in v8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.0-beta.6
Development

Successfully merging this pull request may close these issues.

None yet

5 participants