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

Document the can-block-on relationship for servo. #18810

Merged
merged 1 commit into from Oct 13, 2017

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented Oct 10, 2017

This PR adds some documentation comments describing the can-block-on relation that is used to ensure deadlock-freedom.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #14704
  • These changes do not require tests because it's documentation

This change is Reviewable

@highfive
Copy link

highfive commented Oct 10, 2017

Heads up! This PR modifies the following files:

  • @paulrouget: components/constellation/constellation.rs
//! * Blocking is transitive (if T1 can block on T2 and T2 can block on T3 then T1 can block on T3)
//! * Nothing can block on itself!
//!
//! There is a wrinkle around the use of IPC channels, since they do not support

This comment has been minimized.

@nox

nox Oct 13, 2017

Member

There is a what? Please reword.

This comment has been minimized.

@asajeffrey

asajeffrey Oct 13, 2017

Author Member

Done.

Copy link
Member

cbrewster left a comment

LGTM.

Glad to have this documented.

//! * Nothing can block on itself!
//!
//! There is a complexity intoduced by IPC channels, since they do not support
//! non-blocking send. This means that as well as `receiver.recv()` blocking, so

This comment has been minimized.

@cbrewster

cbrewster Oct 13, 2017

Member

nit: This means that as well as receiver.recv() blocking, sender.send(data) can also block when the IPC buffer is full.

This comment has been minimized.

@asajeffrey

asajeffrey Oct 13, 2017

Author Member

Fixed.

@asajeffrey asajeffrey force-pushed the asajeffrey:constellation-document-blocking branch from 6963d9c to 01e17cf Oct 13, 2017
@asajeffrey
Copy link
Member Author

asajeffrey commented Oct 13, 2017

@bors-servo r=cbrewster

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2017

📌 Commit 01e17cf has been approved by cbrewster

@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2017

Testing commit 01e17cf with merge 4592e6e...

bors-servo added a commit that referenced this pull request Oct 13, 2017
…cbrewster

Document the can-block-on relationship for servo.

<!-- Please describe your changes on the following line: -->

This PR adds some documentation comments describing the can-block-on relation that is used to ensure deadlock-freedom.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #14704
- [X] These changes do not require tests because it's documentation

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/18810)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 13, 2017

@bors-servo bors-servo merged commit 01e17cf into servo:master Oct 13, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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.

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