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

Wait for Eos message on the bus for signalling global end-of-stream #152

Merged
merged 1 commit into from Oct 12, 2018

Conversation

@sdroege
Copy link
Contributor

sdroege commented Oct 12, 2018

Each appsink will signal eos individually and considering the first
one's eos as the global end-of-stream might cause the pipeline to be
shut down before all other appsinks are done processing their data.

The Eos message on the bus is only sent once all sinks are
end-of-stream.

Fixes #151


Untested but should fix the race condition. I've also added the line to send something to the mpsc sender because a few lines below a comment claims that we // Wait until we get an error or EOS., and only the error cases were covered.

@sdroege sdroege force-pushed the sdroege:eos-once branch from 45a248c to e6e0090 Oct 12, 2018
Each appsink will signal eos individually and considering the first
one's eos as the global end-of-stream might cause the pipeline to be
shut down before all other appsinks are done processing their data.

The Eos message on the bus is only sent once *all* sinks are
end-of-stream.

Fixes #151
@sdroege sdroege force-pushed the sdroege:eos-once branch from e6e0090 to 5d34b34 Oct 12, 2018
@Manishearth
Copy link
Member

Manishearth commented Oct 12, 2018

This works locally, I can no longer reproduce the crash.

@Manishearth Manishearth merged commit 99ac127 into servo:master Oct 12, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ferjm ferjm mentioned this pull request Oct 15, 2018
4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.