classification
Title: Make GIL acquire/release functions noops if PyEval_Init
Type: Stage:
Components: Interpreter Core Versions: Python 2.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: gustavo, ncoghlan, tim.peters
Priority: normal Keywords:

Created on 2004-08-18 12:07 by gustavo, last changed 2004-10-11 01:48 by tim.peters. This issue is now closed.

Files
File name Uploaded Description Edit
python.diff gustavo, 2004-08-18 12:07 Patch
Messages (6)
msg22112 - (view) Author: Gustavo J. A. M. Carneiro (gustavo) * Date: 2004-08-18 12:07
The patch makes GIL acquire/release functions
idempotent if PyEval_InitThreads hasn't been called.

Motivation for this change can be found in my post to
python-dev, here: 

http://mail.python.org/pipermail/python-dev/2004-August/047870.html
http://mail.python.org/pipermail/python-dev/2004-August/047871.html

Some feedback here:
http://mail.python.org/pipermail/python-dev/2004-August/047949.html
msg22113 - (view) Author: Gustavo J. A. M. Carneiro (gustavo) * Date: 2004-09-28 14:25
Logged In: YES 
user_id=908

Can someone please take a look at this patch?  It's fairly
simple, but very important.
msg22114 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2004-10-10 15:47
Logged In: YES 
user_id=1038590

The patch doesn't apply cleanly to current CVS (no actual
conflict that I can see - just the patch machinery getting
confused by a cautionary comment added to the top of pystate.c)

Inspecting the patch manually, it looks like it does the
right thing, given that Python instructs that Py_Initialise
and PyEval_InitThreads *both* be called prior while the
application is still single-threaded.

However, _PyThread_Started seems like an odd bit of code -
prior to this patch, it is set (initialised to 0 in
pythonrun.c, set to 1 by PyEval_InitThreads), but never
referenced anywhere (despite the comment saying that it is a
"Flag for Py_Exit").

It's also not part of any API (even an internal one). It is
defined in pythonrun.c, then declared as extern in ceval.c.
The patch adds another extern declaration in pystate.c

It would seem better (and only slightly more work) to add a
real API (such as adding PyThread_IsInitialized() to
pythread.h), rather than entrenching further use of
_PyThread_Started.
msg22115 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2004-10-10 17:08
Logged In: YES 
user_id=1038590

Patch #1044089 has been created with an alternate
implementation of the same concept. Rather than using
_PyThread_Started, it:

- uses interpreter_lock within ceval.c, which matches the
existing ceval.c code that uses the existence of the GIL to
determine if PyEval_InitThreads has been called
- adds a new API function PyEval_ThreadsInitialized which
allows external code to check if the GIL exists or not
- uses this new API in the PyGILState_* functions

The same functions get modified as in the patch attached
here - the only difference is in how the code asks the
question "Has PyEval_InitThreads been called?"
msg22116 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2004-10-10 17:12
Logged In: YES 
user_id=31435

Sorry, I'm opposed to this patch.  Slowing every lock 
operation in the core by an additional test+branch makes 
everyone pay for what you want, and muddies semantics 
that haven't changed since 1991 <wink>.

If you need to do something here, it would be better to go 
back to your first python-dev msg's approach.  Note that the 
core never calls lock operations unless ceval.c's 
interperter_lock is non-NULL.  That's how *it* avoids the 
expense of lock fiddling so long as only one thread is in use.  
It's not a goal to make that easy to spell (we're coding in C 
here, not Python).  It may be a goal to expose enough 
machinery so that motivated extension module authors can 
endure the same kind of pain.
msg22117 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2004-10-11 01:48
Logged In: YES 
user_id=31435

Rejected in favor of 1044089.
History
Date User Action Args
2004-08-18 12:07:16gustavocreate