Issue 1567: Patch for new API method _PyImport_ImportModuleNoLock(char *name) (original) (raw)

Created on 2007-12-07 13:24 by christian.heimes, last changed 2022-04-11 14:56 by admin. 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
2022-04-11 14:56:28 admin set github: 45908
2008-01-04 13:14:41 christian.heimes set status: open -> closedresolution: fixed
2008-01-03 22🔞50 christian.heimes set messages: +
2008-01-03 17:57:43 gvanrossum set messages: +
2008-01-03 16:45:31 christian.heimes set files: - import_nolock.diff
2008-01-03 16:45:26 christian.heimes set files: + trunk_import_noblock.patchmessages: +
2008-01-03 00:15:01 gvanrossum set messages: +
2007-12-12 16:42:41 kbk set nosy: + kbkmessages: + components: - IDLE
2007-12-07 13:24:25 christian.heimes set assignee: christian.heimestype: behaviorcomponents: + IDLE, Interpreter Core
2007-12-07 13:24:04 christian.heimes create