Skip to content

Enforce the HUB lock order#237

Merged
bors[bot] merged 2 commits intogfx-rs:masterfrom
kvark:lock-order
Jul 5, 2019
Merged

Enforce the HUB lock order#237
bors[bot] merged 2 commits intogfx-rs:masterfrom
kvark:lock-order

Conversation

@kvark
Copy link
Copy Markdown
Member

@kvark kvark commented Jun 26, 2019

Fixes #66
cc @jrmuizel @m4b

@kvark kvark requested a review from grovesNL June 26, 2019 03:28
@kvark kvark changed the title [WIP] Enforce the HUB lock order Enforce the HUB lock order Jun 26, 2019
@kvark
Copy link
Copy Markdown
Member Author

kvark commented Jun 26, 2019

This doesn't look great at all, unfortunately. Neither the implementation of Token, or the use of it in locking things. But it's the best thing we can do at the moment, and I think it's worth landing.
Also, I tested all the apps I have, this PR works for me.

/// If type A implements `Access<B>`, that means we are allowed to proceed
/// with locking resource `B` after we lock `A`.
///
/// The impmenentations basically describe the edges in a directed graph
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: implementations

// Before destruction, a resource is expected to have the following strong refs:
// 1. in resource itself
// 2. in the device tracker
// 3. in this list
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to add a fourth item to this list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, no :)
once it has 4 references, it's no longer a resource "before destruction"

@kvark kvark force-pushed the lock-order branch 2 times, most recently from 80701f4 to 622f686 Compare July 2, 2019 17:21
@kvark
Copy link
Copy Markdown
Member Author

kvark commented Jul 2, 2019

@grovesNL fixed the typo and CI, PTAL!

@kvark
Copy link
Copy Markdown
Member Author

kvark commented Jul 4, 2019

@grovesNL rebased and resolved the conflict. Would you want to finish this review?

Copy link
Copy Markdown
Collaborator

@grovesNL grovesNL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

bors bot added a commit that referenced this pull request Jul 5, 2019
237: Enforce the HUB lock order r=grovesNL a=kvark

Fixes #66
cc @jrmuizel @m4b

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Jul 5, 2019

Build succeeded

@bors bors bot merged commit 1ed15f9 into gfx-rs:master Jul 5, 2019
@kvark kvark deleted the lock-order branch July 5, 2019 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Path to avoid deadlocks

2 participants