[Python-bugs-list] [ python-Bugs-683658 ] PyErr_Warn may cause import deadlock

[SourceForge.net]

Bugs item #683658, was opened at 2003-02-10 11:26
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=683658&group_id=5470

Category: Python Interpreter Core
Group: Python 2.3
Status: Open
Resolution: None
Priority: 6
Submitted By: Mark Hammond (mhammond)
Assigned to: Mark Hammond (mhammond)
Summary: PyErr_Warn may cause import deadlock

Initial Comment:
PyErr_Warn() does an implicit import.  Thus, if
PyErr_Warn() is called on a thread while the main
thread holds the import lock, and the main thread then
subsequently waits for the child thread, Python will
deadlock.  The builtin 'apply' now calls PyErr_Warn(),
so simply calling 'apply' on another thread may cause
the deadlock.

Attaching a sample case.  Executing 'python -c "import
hang_apply"' will cause Python to deadlock.  Commenting
out the call to "apply" in that file will prevent the
deadlock (even though the apply() in question is
effectively a no-op)

The example is a little contrived, but is extracted
from real code of mine that saw this hang.  The code
path is:

* Main thread acquires import lock to import the module.
* Module import spawns a new thread.  This new thread
calls apply()
* builtin_apply() calls PyErr_Warn
* PyErr_Warn does an import of the warnings module, and
attempts to acquire the import lock.
* Main thread still waiting for child thread to
complete, but still holds import lock.

Note that removing the call to PyErr_Warn() in
builtin_apply also solves this particular hang -
however, it seems like this is a potential time bomb. 
A potential solution would be to prevent PyErr_Warn
from doing an import - this would mean importing
'warnings' at startup, and keeping a static reference
in errors.c.  Other thoughts welcome.


----------------------------------------------------------------------

>Comment By: Mark Hammond (mhammond)
Date: 2003-02-16 10:17

Message:
Logged In: YES 
user_id=14198

Yes, the linecache import was the problem.  I abandonded
that approach mainly to avoid yet another import at startup.
 Should 'warnings' get new features, all required imports in
the future will also need to be imported at process startup.
 It also struck me as a time bomb.

I will make an alternative patch that moves the imports.


----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-02-16 08:01

Message:
Logged In: YES 
user_id=6380

Oh, and the import of linecache by warnings can be put at
the top of warnings, if that's an issue.

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-02-16 02:46

Message:
Logged In: YES 
user_id=6380

No time to review everything here, but maybe PyErr_Warn
should not do an import? The import could be done at startup
time (when site.py is also imported) and saved in the
interpreter state.

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2003-02-15 16:02

Message:
Logged In: YES 
user_id=14198

The best I can come up with here is exposing the import lock
to the C API, with a "wait" flag.

This will allow a C function to reliably determine if an
"import" work block, or acquire the lock if not.  It can
then complete its import before releasing the lock.  If the
import would block, then it can take whatever action is
necessary - in the case of PyErr_Warn, it dumps the warning
to stdout.

Attaching a patch.  It exposes (to the core, but not in
headers) two functions:

extern int PyImport_LockImport(int wait);
extern void PyImport_UnlockImport(void);

PyErr_Warn then uses these.

If we do go this route, it makes sense to make these
functions truly public (ie, add them to the headers), and
cleanup import.c appropriately.  I didn't do this in the
interests of keeping the patch small so it can be easily
digested.

Comments?  Other ideas?

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2003-02-14 16:16

Message:
Logged In: YES 
user_id=14198

This simple strategy doesn't work - avoiding the import of
'warnings' works fine, until 'warnings' imports 'linecache'!

I'll look at how we can simply throw the warning away if a
deadlock would occur.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-02-12 10:23

Message:
Logged In: YES 
user_id=33168

I can't think of any other/better solution.  Any idea if
there are there other ticking bombs like this?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=683658&group_id=5470