8000 bpo-33316: PyThread_release_lock always fails by native-api · Pull Request #6541 · python/cpython · GitHub
[go: up one dir, main page]
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
zooba merged 3 commits into from
Feb 2, 2019
Merged

bpo-33316: PyThread_release_lock always fails #6541

zooba 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
ned-deily 8000 commented Jun 8, 2018

@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
serhiy-storchaka commented Dec 8, 2018

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
vstinner commented Jan 10, 2019

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
miss-islington commented Feb 2, 2019

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
bedevere-bot commented Feb 2, 2019

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
0