Skip to content

bpo-33316: PyThread_release_lock always fails #6541

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 2, 2019
Merged

bpo-33316: PyThread_release_lock always fails #6541

merged 3 commits into from
Feb 2, 2019

Conversation

native-api
Copy link
Contributor

@native-api native-api commented Apr 20, 2018 •
edited by bedevere-bot
Loading

@ned-deily
Copy link
Member

@zooba Is this ready to merge?

@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Dec 8, 2018
@serhiy-storchaka serhiy-storchaka requested a review from a team December 8, 2018 10:09
@serhiy-storchaka
Copy link
Member

Could you add any tests?

@native-api
Copy link
Contributor Author

native-api commented Dec 9, 2018 •
edited
Loading

I don't readily see how it's possible to test this. There's no error code or anything at PyThread_release_lock level, only an error message, and only if I patch the source (set thread_debug to 1 at https://github.com/python/cpython/blob/master/Python/thread.c#L48).

I could test LeaveNonRecursiveMutex directly. But current tests only test the public API, and PyThread_* and *Mutex are not parts of it, so it's unclear where I should put the tests and what spec I should test against.

I can think of something myself but would like to receive some feedback so that I don't have to redo it afterwards.

@vstinner
Copy link
Member

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

@zooba
Copy link
Member

zooba commented Feb 2, 2019

We didn't have a test before, and this is code that has always been incorrect yet without apparent impact.

@zooba zooba merged commit 05e9221 into python:master Feb 2, 2019
@miss-islington
Copy link
Contributor

Thanks @native-api for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 2, 2019
@native-api @miss-islington
bpo-33316: PyThread_release_lock always fails (pythonGH-6541)
fe14a7d 
Use correct interpretation of return value from APIs.
(cherry picked from commit 05e9221)

Co-authored-by: native-api <ivan_pozdeev@mail.ru>
@bedevere-bot
Copy link

GH-11737 is a backport of this pull request to the 3.7 branch.

@native-api native-api deleted the PyThread_release_lock branch February 2, 2019 16:26
miss-islington added a commit that referenced this pull request Feb 2, 2019
@miss-islington @native-api
bpo-33316: PyThread_release_lock always fails (GH-6541)
c851dfc 
Use correct interpretation of return value from APIs.
(cherry picked from commit 05e9221)

Co-authored-by: native-api <ivan_pozdeev@mail.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@native-api @ned-deily @serhiy-storchaka @vstinner @zooba @miss-islington @bedevere-bot @the-knights-who-say-ni