Lock.release(): reorder code to avoid token overwrite #489
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* assignment to self.token was not protected by the lock, so the value could get overwritten * do_release() now has an expected_token parameter that receives the old token value * NOTE: this only fixes the issue for locks that do not have timeouts
andymccurdy
added a commit
that referenced
this pull request
Jun 5, 2014
Lock.release(): reorder code to avoid token overwrite
|
Ah, ya that makes sense. I hadn't spent much time thinking about the use cases where multiple Python threads used a shared lock instance. Let me know what you come up with about the timeout issue. |
|
Just re-read #416. I see what you're talking about now -- when thread-0 calls release() after running over the timeout it uses the token that thread-1 (who now holds the lock) generated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the recent lock rewrite reintroduced a data stomp bug similar to the one fixed in #384. this change should patch the problem for locks that do not have timeouts, which is the same behavior the library had pre-rewrite. here is a gist demonstrating the issue. before applying this change, i was able to get double-release exceptions about 50% of the time on a linux VM with a sleepTime of 0.15s, and 100% of the time on windows with a sleepTime of 0.1s. i did all of the above tests with python 2.7.5.
unfortunately, the bug described in #416 is still present, which is closely related to this code. i'll open a separate issue for that.