A finer grained import lock

[neologix_free.fr]

Sorry for the delay, forgot about this one.

This version looks safe (apart from the limitation documented in the code that
only one potential lock acquirer thread is chosen for cycle detection).
I find it a bit complex, though.

I didn't follow the recent discussion on importlib, but if there's a plan to
replace most of import machinery by importlib, wouldn't it be easier - if at all
possible - to implement this in Python at that time?

http://bugs.python.org/review/9260/diff/3916/12928
File Python/import.c (right):

http://bugs.python.org/review/9260/diff/3916/12928#newcode153
Python/import.c:153: init_import_lock(lock);
Shouldn't the lock be cleared (I have the impression that the previous
lock->lock is leaked)?

http://bugs.python.org/review/9260/diff/3916/12928#newcode167
Python/import.c:167: _get_module_lock(PyObject *fullname)
I usually declare pointer arguments const if they're not modified.

http://bugs.python.org/review/9260/diff/3916/12928#newcode206
Python/import.c:206: return r;
I'm not familiar with capsules, but shouldn't capsule be decref'ed ?

[AntoinePitrou]

> This version looks safe (apart from the limitation documented in the code that
> only one potential lock acquirer thread is chosen for cycle detection).
> I find it a bit complex, though.

Yes, it's a bit complex. Do you have any idea to make it simpler?

> I didn't follow the recent discussion on importlib, but if there's a plan to
> replace most of import machinery by importlib, wouldn't it be easier - if at
all
> possible - to implement this in Python at that time?

I'm not sure the algorithm would be simpler as expressed in Python. We would
avoid the capsule mess, though (but we would also need support for after fork
reinitialization of locks :-)).
Using importlib by default is blocking on performance issues. I'm having a
problem with directory modification times here:
http://bugs.python.org/issue14043

[AntoinePitrou]

http://bugs.python.org/review/9260/diff/3916/12928
File Python/import.c (right):

http://bugs.python.org/review/9260/diff/3916/12928#newcode153
Python/import.c:153: init_import_lock(lock);
On 2012/02/18 13:04:52, Charles-François Natali wrote:
> Shouldn't the lock be cleared (I have the impression that the previous
> lock->lock is leaked)?

In the old _PyImport_ReInitLock() implementation, the global "import_lock"
variable is simply overwritten.
Is it safe to clear a lock from a child process?

http://bugs.python.org/review/9260/diff/3916/12928#newcode167
Python/import.c:167: _get_module_lock(PyObject *fullname)
On 2012/02/18 13:04:52, Charles-François Natali wrote:
> I usually declare pointer arguments const if they're not modified.

Hmm, true. On the other hand, whether or not the inside of a PyObject can be
modified is pretty much an implementation detail (think cached data such as a
string's hash value).

http://bugs.python.org/review/9260/diff/3916/12928#newcode206
Python/import.c:206: return r;
On 2012/02/18 13:04:52, Charles-François Natali wrote:
> I'm not familiar with capsules, but shouldn't capsule be decref'ed ?

Oops, indeed.

[loewis]

http://bugs.python.org/review/9260/diff/4829/17079
File Lib/importlib/_bootstrap.py (right):

http://bugs.python.org/review/9260/diff/4829/17079#newcode1115
Lib/importlib/_bootstrap.py:1115: lock.acquire()
Why does that not do deadlock avoidance? I still fail to understand how the
deadlock avoidance mechanism actually affects the importing. I.e. what precisely
happens when you run into a deadlock to the import that is in progress?

[AntoinePitrou]

http://bugs.python.org/review/9260/diff/4829/17079
File Lib/importlib/_bootstrap.py (right):

http://bugs.python.org/review/9260/diff/4829/17079#newcode1115
Lib/importlib/_bootstrap.py:1115: lock.acquire()
On 2012/05/05 22:11:16, loewis wrote:
> Why does that not do deadlock avoidance? I still fail to understand how the
> deadlock avoidance mechanism actually affects the importing. I.e. what
precisely
> happens when you run into a deadlock to the import that is in progress?

Deadlock avoidance is done in _lock_unlock_module(), which is invoked when a
module object is already present in sys.modules.

When a deadlock is detected, the module lock is simply not taken, meaning that
the thread sees an incomplete module, exactly like in other circular imports.