[C API] Remove PyEval_AcquireLock() and PyEval_ReleaseLock() functions

[vstinner]

BPO	39998
Nosy	@vstinner, @serhiy-storchaka
PRs	bpo-39998: Remove PyEval_AcquireLock() and PyEval_ReleaseLock() #19048

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-03-20.17:31:26.752>
created_at = <Date 2020-03-17.23:39:02.379>
labels = ['expert-C-API', '3.9']
title = '[C API] Remove PyEval_AcquireLock() and PyEval_ReleaseLock() functions'
updated_at = <Date 2020-03-20.17:31:26.751>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2020-03-20.17:31:26.751>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-03-20.17:31:26.752>
closer = 'vstinner'
components = ['C API']
creation = <Date 2020-03-17.23:39:02.379>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 39998
keywords = ['patch']
message_count = 8.0
messages = ['364487', '364488', '364678', '364682', '364684', '364689', '364690', '364691']
nosy_count = 2.0
nosy_names = ['vstinner', 'serhiy.storchaka']
pr_nums = ['19048']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39998'
versions = ['Python 3.9']

[vstinner]

The PyEval_AcquireLock() and PyEval_ReleaseLock() functions are misleading and deprecated since Python 3.2.

bpo-10913 deprecated them:

commit 5ace8e9
Author: Antoine Pitrou <solipsis@pitrou.net>
Date: Sat Jan 15 13:11:48 2011 +0000

Issue bpo-10913: Deprecate misleading functions PyEval_AcquireLock() and
PyEval_ReleaseLock().  The thread-state aware APIs should be used instead.

It's now time to remove them!

I *discovered* these functions while working on bpo-39984. Previously, I never ever used them nor really see them. I only made refactoring them in their code, without paying attention to them.

[vstinner]

What's New In Python 3.2 says:
"The misleading functions PyEval_AcquireLock() and PyEval_ReleaseLock() have been officially deprecated. The thread-state aware APIs (such as PyEval_SaveThread() and PyEval_RestoreThread()) should be used instead."
https://docs.python.org/dev/whatsnew/3.2.html#porting-to-python-3-2

[vstinner]

PyEval_AcquireLock() is declared with Py_DEPRECATED(3.2) since Python 3.7

PyEval_ReleaseLock() is not declared with Py_DEPRECATED(), but PyEval_ReleaseLock() cannot be used without PyEval_AcquireLock().

[serhiy-storchaka]

We cannot just remove functions from stable ABI. We can undocument them, remove their declaration from header files, but we can't remove the implementation. Just make them always failing.

[serhiy-storchaka]

See for example _PyTrash_deposit_object.

[vstinner]

Hum, I found multiple projects using PyEval_AcquireLock() and PyEval_ReleaseLock(). Many of them look abandonned. But a few were modified earlier than 1 year old.

--

ntripcaster2: Latest commit on Jun 2019

https://github.com/rinex20/ntripcaster2/blob/ef45763c4c0b063e46ef3bbfc9d63bafcd76e421/src/interpreter.c

Example:

  PyEval_AcquireLock ();

  maininterpreterstate = mainthreadstate->interp;
  newthreadstate = PyThreadState_New (maininterpreterstate);

  PyEval_ReleaseLock ();

--

I found usage of PyEval_AcquireLock() in an old version of pygame (1.9.1, latest is 1.9.6):

    PyEval_AcquireLock ();
    oldstate = PyThreadState_Swap (helper->thread);
    ...
    result = PyObject_CallFunction (helper->tell, NULL);
    ...
    PyThreadState_Swap (oldstate);
    PyEval_ReleaseLock ();

https://github.com/z-pan/pygame_tankScout/blob/7977cc6756e948c37cb4aa56fb1009a74288b65c/pygame-1.9.1release/src/rwobject.c

pygame changed to "PyGILState_Ensure() ... PyGILState_Release()" instead.

--

giljoy (Latest commit in 2013): it seems to override symloads using LD_PRELOAD to measure time when the GIL is acquired and released.

https://github.com/itamarst/giljoy/blob/master/giljoy.c

--

xmlbus: Latest commit in 2014.

https://github.com/olger/xmlbus/blob/148692997a481d10827a1aa86dc0ed3d70d84209/server/xmlbusd/pyrunner/pyrunner.c

Example:

	PyEval_AcquireLock();
	mainInterpreterState = mainThreadState->interp;
	PyThreadState_Swap(mainThreadState);

	myThreadState = PyThreadState_New(mainInterpreterState);
	PyEval_ReleaseLock();

[vstinner]

(Trashcan is somehow off-topic here, but let me comment anyway ;-))

See for example _PyTrash_deposit_object.

I know that this one is kept for ABI backward compatibility... but the TRASHCAN API is excluded from the limited API. So I'm not sure that it is worth it to keep _PyTrash_deposit_object() in the ABI.

I modified the TRASHCAN API in Python 3.9 to no longer leak implementation details (access PyThreadState structure fields). The implementation now only uses function calls.

=> commit 38965ec

[vstinner]

Oh no, PyEval_AcquireLock() and PyEval_ReleaseLock() are part of the limited C API (and so the stable ABI). Sadly, we have to keep them. I close the issue as rejected.

We cannot just remove functions from stable ABI.

Alright, sadly it's part of the limited C API :-(

We can undocument them, remove their declaration from header files, but we can't remove the implementation. Just make them always failing.

Since I found a few projects using these functions, I'm no longer sure that it's worth it to remove these functions.

Continuing to maintain these functions is not really a major maintenance burden right now. So I simply close this issue.

If someone disagree, you can propose an implementation of Serhiy's suggestion.