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.

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?

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.

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?

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.

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.

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.

Logged In: YES 
user_id=14198

Checking in a patch that avoids the import, thereby avoiding
the hang.  As suggested, the "linecache" import in warnings
had to be moved.

Even though I checked it in, I attach the patch here, and
have assigned it to Guido for review.  If happy, just mark
it as closed.

/cvsroot/python/python/dist/src/Python/errors.c,v  <--  errors.c
new revision: 2.76; previous revision: 2.75
/cvsroot/python/python/dist/src/Python/pythonrun.c,v  <-- 
pythonrun.c
new revision: 2.178; previous revision: 2.177
/cvsroot/python/python/dist/src/Lib/warnings.py,v  <-- 
warnings.py
new revision: 1.19; previous revision: 1.18

Logged In: YES 
user_id=6380

I'm not happy with this solution, since it breaks for
multiple interpreters. So keeping this open. Will think
about what to do instead after 2.3a2 is released.

Logged In: YES 
user_id=92689

Here's an issue with the patch I just ran into: it seems this patch causes the first Python module to be imported to be warnings.py instead of site.py. This means site.py doesn't have full control of the import mechanism anymore, as all modules loaded by warnings.py will already be loaded. Can't the PyModule_WarningsModule global be set _after_ site.py has run?

Use case: I'm building standalone applications in which the import bootstrapping happens in a custom site.py, assuming it will be the first Python module that will be imported. It sets up sys.path appropriately. Now it turns out that when my site.py gets run (eg.) os.py is already loaded from my system-wide installation, instead of from the zip file embedded in the application. This means I can't reliably test whether my built app really works when there's no Python installation available.

Logged In: YES 
user_id=6380

Good point. But loading site.py first means that warnings
issued as a result of importing site.py are not handled
properly (they will be dumped to stderr, which is better
than nothing). Well, I have to revisit this anyway; I'll
make sure to take this issue into account as well.

Logged In: YES 
user_id=92689

Additional info regarding the current situation (with the patch in place):
if warnings.py can't be imported before site.py is run, _all_ warnings issued from C will be printed to stderr, even if warnings.py _can_ be imported later.

I think it's fair to assume that any warning issued during the import of site.py signifies that something is pretty broken, so having them dumped to stderr isn't all that bad.

If you and Mark agree, I'll move the import of warnings.py to after initsite().

Logged In: YES 
user_id=6380

OK, as a stopgap measure that sounds like a good idea.

(I wonder if -S should also suppress loading warnings? Or is
that too drastic?)

Logged In: YES 
user_id=92689

Only if PyErr_Warn() would still also attempt to do the import if PyModule_WarningsModule is NULL. But that means that deadlocks can still occur when -S is used. But I _do_ like it that no file system imports are done by Python itself with -S. And I actually don't like it at all that warnings.py always gets imported, even if it's never needed.

Logged In: YES 
user_id=6380

OK, implement that.

Logged In: YES 
user_id=92689

Well, since you said the issue needs to be revisited anyway, I'll just leave it at moving the warnings.py import after initsite().

What I don't like is that warnings.py is imported preemptively to begin with, not just with -S. Whether that's resolvable I don't know, but if it is, it would be better than not importing warnings.py with -S.

Logged In: YES 
user_id=31392

Did this get resolved for 2.3 or delayed to 2.4?

Logged In: YES 
user_id=92689

This is from the CVS log of pythonrun.c:

revision 2.179
date: 2003/02/25 20:25:12;  author: jvr;  state: Exp;  lines: +2 -2
Addendum to #683658:
import warnings.py _after_ site.py has run. This ensures that 
site.py is again the first .py to be imported, giving it back full 
control over sys.path.

I'm quite confident this is good enough for 2.3, but the issue 
should be revisited at some point, so I'm not sure we should close 
this bug. I'll lower the priority.

Is this still an issue for 2.5? If Python 2.5 may still dead lock then
somebody should back port my PyImport_ImportModuleNoBlock() function.

No, backporting that to 2.5 isn't an option; it introduces a new feature
and breaks binary backwards compatibility by removing the
PyImport_ImportModuleEx entry point from the executable (turning it into
a macro).

Let's close this unless there's a response from one of the original
stakeholders within a week (i.e. by Jan 11 2008).