classification
Title: Windows: PyThread_release_lock always fails
Type: behavior Stage: resolved
Components: Interpreter Core, Windows Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: Ivan.Pozdeev, miss-islington, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2018-04-20 00:18 by Ivan.Pozdeev, last changed 2019-02-02 17:15 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 6541 merged Ivan.Pozdeev, 2018-04-20 00:25
PR 11737 merged miss-islington, 2019-02-02 16:23
PR 11737 merged miss-islington, 2019-02-02 16:23
PR 11737 merged miss-islington, 2019-02-02 16:23
Messages (7)
msg315497 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2018-04-20 00:18
In win7 x64 debug mode with thread_debug=1, every PyThread_release_lock() is accompanied with a message on stderr:

Could not PyThread_release_lock(<id>) error: 0
msg315670 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-04-23 16:40
Looks okay, but I wonder whether the implementation of PyMUTEX_UNLOCK should return something other than zero? What is it meant to return?
msg315714 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2018-04-24 21:36
In all PyMUTEX_* and most PyCOND_* functions, all the underlying APIs are void. So there's no point for the functions to return anything other than as convention.

A comment at the start of condvar.h says: "APIs generally return 0 on success and non-zero on error".
In Python\ceval_gil.h:57, there are boilerplace checker macros for them
(which will never fire).

So I guess this is done for uniformity.
Regardless, PyMUTEX_UNLOCK and PyMUTEX_LOCK at least are called both with and without checking the return value.


So I would drop the pretense and make these fns void.

---

That said, the reported error still persists after the fix.
The code confusing about which value LeaveNonRecursiveMutex should return on success. Pushed another commit, confirmed to work now.
msg316661 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2018-05-15 14:57
> So I would drop the pretense and make these fns void.

Scratch that. In POSIX, they are #define'd as pthread fns which do return an error code.

So, nothing more to do here.
msg317259 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2018-05-22 05:57
> So, nothing more to do here.

In case that was cryptic, it means: the PR can be acted upon, no other changes are needed.
msg334751 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-02-02 16:22
New changeset 05e922136a3286893bd489a8f2ecfa0dba4da368 by Steve Dower (native-api) in branch 'master':
bpo-33316: PyThread_release_lock always fails (GH-6541)
https://github.com/python/cpython/commit/05e922136a3286893bd489a8f2ecfa0dba4da368
msg334754 - (view) Author: miss-islington (miss-islington) Date: 2019-02-02 16:45
New changeset c851dfc99b28c7335d42abd8250bb77c11ce23c0 by Miss Islington (bot) in branch '3.7':
bpo-33316: PyThread_release_lock always fails (GH-6541)
https://github.com/python/cpython/commit/c851dfc99b28c7335d42abd8250bb77c11ce23c0
History
Date User Action Args
2019-02-02 17:15:27steve.dowersetstatus: open -> closed
assignee: steve.dower
resolution: fixed
stage: backport needed -> resolved
2019-02-02 16:45:52miss-islingtonsetnosy: + miss-islington
messages: + msg334754
2019-02-02 16:26:41steve.dowersetstage: patch review -> backport needed
versions: - Python 3.6
2019-02-02 16:23:26miss-islingtonsetpull_requests: + pull_request11644
2019-02-02 16:23:20miss-islingtonsetpull_requests: + pull_request11643
2019-02-02 16:23:12miss-islingtonsetpull_requests: + pull_request11642
2019-02-02 16:22:57steve.dowersetmessages: + msg334751
2018-05-22 05:57:34Ivan.Pozdeevsetmessages: + msg317259
2018-05-15 14:57:21Ivan.Pozdeevsetmessages: + msg316661
2018-05-02 21:52:47vstinnersettitle: PyThread_release_lock always fails -> Windows: PyThread_release_lock always fails
2018-04-24 21:36:56Ivan.Pozdeevsetmessages: + msg315714
2018-04-23 16:40:10steve.dowersetmessages: + msg315670
2018-04-20 00:25:42Ivan.Pozdeevsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6237
2018-04-20 00:18:56Ivan.Pozdeevcreate