Making test_winreg passes.#718
Merged
slozier merged 8 commits intoIronLanguages:masterfrom Jan 11, 2020
Merged
Conversation
slozier
reviewed
Jan 9, 2020
Contributor
slozier
left a comment
slozier
left a comment
There was a problem hiding this comment.
Thanks for the PR!
For whatever reason test_difflib and test_userlist timed out during CI. Rerunning now to see if it was a glitch, otherwise will need to investigate.
Would be nice to include the test from #410 in Tests/test_regressions.py. Did you try running test_winreg to see if this unblocks it?
Contributor
Author
|
Okay..., this PR is completely messed up. I’m sorry. I will fix it. |
in-code-i-trust
force-pushed
the
implement-thread-sentinel-lock-and-acquire-timeout
branch
from
db62aad to
51409e0
Compare
Contributor
Author
|
@slozier So here are what i've done in this PR.
|
in-code-i-trust
changed the title
_set_sentinel and timeout for lock.acquire()test_winreg passes.
This behavior is required to make `test_winreg` passes.
This test was introduced in GitHub issue 410 and resolved in commit 972a7bd.
in-code-i-trust
force-pushed
the
implement-thread-sentinel-lock-and-acquire-timeout
branch
from
e4559b2 to
5621acf
Compare
slozier
approved these changes
Jan 11, 2020
Contributor
slozier
left a comment
slozier
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for figuring out the issue and making test_winreg pass!
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
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.
Hi,
I wrote a patch to resolve #410.
The code shown in #410 causes a deadlock because
_set_sentinel()is not implemented asthreading.pyexpects; an lock object returned by_set_sentinel()should be released on its thread exit.I also implemented a lacking feature, timeout for
acquire()as well.According to the documentation, timeout is only supported on blocking acquire.