classification
Title: Patch for new API method _PyImport_ImportModuleNoLock(char *name)
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: christian.heimes, gvanrossum, kbk
Priority: normal Keywords:

Created on 2007-12-07 13:24 by christian.heimes, last changed 2008-01-04 13:14 by christian.heimes. This issue is now closed.

Files
File name Uploaded Description Edit
trunk_import_noblock.patch christian.heimes, 2008-01-03 16:45
Messages (6)
msg58272 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-12-07 13:24
The patch adds a new API method _PyImport_ImportModuleNoLock(const char
*name). It works mostly like PyImport_ImportModule except it does not
block. It tries to fetch the module from sys.modules first and falls
back to PyImport_ImportModule UNLESS the import lock is held by another
thread.

It fixes several issues related to dead locks when a thread uses a
function that imports a module but another thread holds the lock. It
doesn't require static caching of modules.

The patch is against py3k but I can backport it to 2.6.
msg58500 - (view) Author: Kurt B. Kaiser (kbk) * (Python committer) Date: 2007-12-12 16:42
Doesn't seem to be IDLE related, removed IDLE tag.
msg59116 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-01-03 00:15
Can you produce a version of the patch that applies cleanly to 2.6? Then
we can apply it there first and merge into 3.0 later.

But I wonder.  It seems this changes every single call to
PyImport_ImportModule() in the core to _PyImport_ImportModuleNoLock(). 
What's the point of the lock at that point?
msg59136 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-01-03 16:45
The patch does't replace all PyImport_ImportModule() calls with the new
function. It replaces only the calls which may occure more than once and
have a fixed name like "time".

I've renamed the function to PyImport_ImportModuleNoBlock(). It should
explain the purpose of the function better. It imports a module without
blocking.
msg59139 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-01-03 17:57
I see.  I think there's still something uncomfortable with this function
though -- it looks in sys.modules, but if it doesn't find it there, it
invokes the full-blown import machinery, which calls the (possibly
overridden) __import__ builtin, which in turn might do a relative import
(at least in 2.6) or any amount of funny business.

The only thing that really worries me here is the possibility of
relative import.  I'm thinking that relative import is really never
meant to be when called from C, so perhaps it would be sufficient to
modify the call to __import__ at the end of PyImport_Import() to add a
5th parameter, zero, to always force absolute import.  A quick concept
test shows that this doesn't break anything, though it is worth checking
the docs.
msg59169 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-01-03 22:18
Guido van Rossum wrote:
> The only thing that really worries me here is the possibility of
> relative import.  I'm thinking that relative import is really never
> meant to be when called from C, so perhaps it would be sufficient to
> modify the call to __import__ at the end of PyImport_Import() to add a
> 5th parameter, zero, to always force absolute import.  A quick concept
> test shows that this doesn't break anything, though it is worth checking
> the docs.

Committed in r59678
I've changed PyImport_Import to always use absolute imports as
requested. The docs didn't promise relative imports. It's probably a
mistake to allow relative imports in the first place.

I've also updated the docs and NEWS.
History
Date User Action Args
2008-01-04 13:14:41christian.heimessetstatus: open -> closed
resolution: fixed
2008-01-03 22:18:50christian.heimessetmessages: + msg59169
2008-01-03 17:57:43gvanrossumsetmessages: + msg59139
2008-01-03 16:45:31christian.heimessetfiles: - import_nolock.diff
2008-01-03 16:45:26christian.heimessetfiles: + trunk_import_noblock.patch
messages: + msg59136
2008-01-03 00:15:01gvanrossumsetmessages: + msg59116
2007-12-12 16:42:41kbksetnosy: + kbk
messages: + msg58500
components: - IDLE
2007-12-07 13:24:25christian.heimessetassignee: christian.heimes
type: behavior
components: + IDLE, Interpreter Core
2007-12-07 13:24:04christian.heimescreate