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 race condition with seek lock by enforcing an ack #289

Merged
merged 4 commits into from Jul 25, 2019

Conversation

@ferjm
Copy link
Member

ferjm commented Jul 25, 2019

No description provided.

@ferjm
Copy link
Member Author

ferjm commented Jul 25, 2019

r? @ceyusa

@@ -664,7 +669,8 @@ impl GStreamerPlayer {
seek_channel.lock().unwrap().sender()
)
);
let ret = seek_channel.lock().unwrap().await();
let (ret, ack_channel) = seek_channel.lock().unwrap().await();
ack_channel.send(()).unwrap();

This comment has been minimized.

@ceyusa

ceyusa Jul 25, 2019

Contributor

shouldn't this ack_channel.send(()).unwrap() be after servosrc_.set_seek_done()???

This comment has been minimized.

@ceyusa

ceyusa Jul 25, 2019

Contributor

I was thinking, rather than juggling with another ipc channel, what about sending a SeekDone message after the set_seek_done()

SeekDone is used to communicate the seek helperst to teardown in Servo, so it should be use for this message.

@ferjm ferjm mentioned this pull request Jul 25, 2019
2 of 3 tasks complete
@ceyusa
Copy link
Contributor

ceyusa commented Jul 25, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2019

📌 Commit 5d24b6f has been approved by ceyusa

@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2019

Testing commit 5d24b6f with merge 1e52744...

bors-servo added a commit that referenced this pull request Jul 25, 2019
Fix race condition with seek lock by enforcing an ack
@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2019

☀️ Test successful - checks-travis
Approved by: ceyusa
Pushing 1e52744 to master...

@bors-servo bors-servo merged commit 5d24b6f into servo:master Jul 25, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@ferjm ferjm deleted the ferjm:seeklock branch Jul 26, 2019
bors-servo added a commit to servo/servo that referenced this pull request Aug 13, 2019
[WIP] Fix HTMLMediaElement seek race condition

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

This depends on servo/media#289 and it's blocked by the servo-media update issue mentioned [here](#23842 (comment))

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23853)
<!-- Reviewable:end -->
bors-servo added a commit to servo/servo that referenced this pull request Aug 13, 2019
Fix HTMLMediaElement seek race condition

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

This depends on servo/media#289 and it's blocked by the servo-media update issue mentioned [here](#23842 (comment))

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23853)
<!-- Reviewable:end -->
bors-servo added a commit to servo/servo that referenced this pull request Aug 14, 2019
Fix HTMLMediaElement seek race condition

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

This depends on servo/media#289 and it's blocked by the servo-media update issue mentioned [here](#23842 (comment))

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23853)
<!-- Reviewable:end -->
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.

None yet

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