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: stream protocols not completing #21733

Merged
merged 1 commit into from Jan 13, 2020
Merged

fix: stream protocols not completing #21733

merged 1 commit into from Jan 13, 2020

Conversation

nornagon
Copy link
Member

Description of Change

It's possible for NodeStreamLoader to receive the "end" event from the source stream while it's still writing to the network service. When that happens, it doesn't immediately clean itself up; instead it notes that the source stream is finished and waits for the write operation to complete. However, it wasn't correctly calling NotifyComplete when finishing the final write operation. This adds that call when appropriate.

Fixes #21018. h/t to @pfrazee for the minimal repro which allowed me to track this down.

Checklist

Release Notes

Notes: Fixed an issue where custom stream protocols would sometimes not complete responses when the data stream ended.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jan 10, 2020
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jan 11, 2020
@nornagon nornagon merged commit d68f17b into master Jan 13, 2020
@release-clerk
Copy link

release-clerk bot commented Jan 13, 2020

Release Notes Persisted

Fixed an issue where custom stream protocols would sometimes not complete responses when the data stream ended.

@nornagon nornagon deleted the fix-stream-protocol branch January 13, 2020 18:28
@trop
Copy link
Contributor

trop bot commented Jan 13, 2020

I have automatically backported this PR to "7-1-x", please check out #21758

@trop trop bot removed the target/7-1-x label Jan 13, 2020
@trop
Copy link
Contributor

trop bot commented Jan 13, 2020

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

@sofianguy sofianguy added this to Fixed in 8.0.0-beta.7 in 8.2.x Jan 16, 2020
@sofianguy sofianguy added this to Fixed in 7.1.10 in 7.2.x Feb 15, 2020
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
7.2.x
Fixed in 7.1.10
8.2.x
Fixed in 8.0.0-beta.7
Development

Successfully merging this pull request may close these issues.

Stream protocols and fs.createReadStream sometimes never complete in Electron 7
3 participants