[#MODPYTHON-2] multiple/redundant interpreter creation

mod_python

multiple/redundant interpreter creation

Created: 14/Dec/04 09:51 PM Updated: 05/Mar/06 01:33 PM

Return to search

Component/s:

None

Affects Version/s:

3.1.3

Fix Version/s:

3.2.7

Time Tracking:

Not Specified

File Attachments:

	File Name	Date Attached	Attached By	Size
	mod_python.c.patch	2004-12-14 09:53 PM	Nicolas Lehuen	2 kB

Environment:

mod_python 3.1.3 + a threaded MPM (observed on Win32 and Mac OS X)

Resolution Date:

19/Dec/04 07:42 PM

Description

« Hide

A small bug in mod_python.c allows the creation of many Python interpreters, where there should be only one. As a result, modules can be loaded multiple times (once per interpreter) and some higher level bugs can occur (beginning with higher memory usage). Graham Dumpleton found the bug in mod_python.c, and I completed the fix with a patch to apache.py (in the import_module) function.

All

Comments

Work Log

Change History

Subversion Commits

Sort Order:

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 14/Dec/04 09:53 PM

Graham's patch.

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 14/Dec/04 09:57 PM

The patch for mod_python.c solves a problem with mod_python creating multiple interpreters with the same name, but there is stil a problem in the mod_python.apache module, since it reloads the same publisher many times.

Indeed, having a look at apache.py, I didn't see any locking done to prevent the same module from being reloaded multiple times concurrently. What we need is to put a lock in the import_module method, so that two threads cannot check the freshness of the publisher simultaneously. This can be done in a rather brutal way by calling imp.acquire_lock() at the beginning of import_module, put a try/finally block around the code of the function and call imp.release_lock() in the finally block :

def import_module(module_name, autoreload=1, log=0, path=None):
    """
    Get the module to handle the request. If
    autoreload is on, then the module will be reloaded
    if it has changed since the last import.
    """

    imp.acquire_lock()
    try:
# previous code of apache.import_module
    finally:
        imp.release_lock()

I tested this, and it works as expected. BTW, maybe it wouldn't work without the patch to mod_python.c, since I could suddenly see two interpreters with their own copy of the mod_python.apache module. But the two fixes are required for it to work properly.

Why did I wrote "in a rather brutal way" above ? Because this locking scheme is indeed brutal. It is a global lock, so each and every thread who want to get its handler must acquire it for a brief amount of time. The problem is that if a handler module is reloaded, it locks each and every other thread during its reloading, even if they should not be handled by the same module. A smarter way to lock would be to use a two-levels locking scheme, as I described in my caching recipe in the Python Cookbook. I can implement it there if it's ok for everybody.

I suggest we solve this promptly with a big lock and try to have a finer locking scheme later on.

[ Show » ]

Nicolas Lehuen added a comment - 14/Dec/04 09:57 PM The patch for mod_python.c solves a problem with mod_python creating multiple interpreters with the same name, but there is stil a problem in the mod_python.apache module, since it reloads the same publisher many times. Indeed, having a look at apache.py, I didn't see any locking done to prevent the same module from being reloaded multiple times concurrently. What we need is to put a lock in the import_module method, so that two threads cannot check the freshness of the publisher simultaneously. This can be done in a rather brutal way by calling imp.acquire_lock() at the beginning of import_module, put a try/finally block around the code of the function and call imp.release_lock() in the finally block : def import_module(module_name, autoreload=1, log=0, path=None): """ Get the module to handle the request. If autoreload is on, then the module will be reloaded if it has changed since the last import. """ imp.acquire_lock() try: # previous code of apache.import_module finally: imp.release_lock() I tested this, and it works as expected. BTW, maybe it wouldn't work without the patch to mod_python.c, since I could suddenly see two interpreters with their own copy of the mod_python.apache module. But the two fixes are required for it to work properly. Why did I wrote "in a rather brutal way" above ? Because this locking scheme is indeed brutal. It is a global lock, so each and every thread who want to get its handler must acquire it for a brief amount of time. The problem is that if a handler module is reloaded, it locks each and every other thread during its reloading, even if they should not be handled by the same module. A smarter way to lock would be to use a two-levels locking scheme, as I described in my caching recipe in the Python Cookbook. I can implement it there if it's ok for everybody. I suggest we solve this promptly with a big lock and try to have a finer locking scheme later on.

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 19/Dec/04 07:42 PM

This should be OK now, but I need other people to compile and test the fix on platforms other than Win32.