Fixed race conditions on setting sys.path when the PythonPath directi…

…ve is

being used as well as problems with infinite extension of path. (MODPYTHON-114)
Stopped directories being added to sys.path multiple times when PythonImport
and PythonPath directive used. (MODPYTHON-147)

Doc/appendixc.tex

@@ -156,6 +156,15 @@ \chapter{Changes from Version (3.2.8)\label{app-changes-from-3.2.8}}
       The \code{PythonImport} directive now uses the
       \code{apache.import_module()} function to import modules to avoid
       reloading problems when same module is imported from a handler.
+    \item
+      (\citetitle[http://issues.apache.org/jira/browse/MODPYTHON-114]{MODPYTHON-114})
+      Fixed race conditions on setting \code{sys.path} when the
+      \code{PythonPath} directive is being used as well as problems with
+      infinite extension of path.
+    \item
+      (\citetitle[http://issues.apache.org/jira/browse/MODPYTHON-147]{MODPYTHON-147})
+      Stopped directories being added to \code{sys.path} multiple times when
+      \code{PythonImport} and \code{PythonPath} directive used.
   \end{itemize}
 
 \chapter{Changes from Version (3.2.7)\label{app-changes-from-3.2.7}}

lib/python/mod_python/apache.py

@@ -27,9 +27,14 @@
 import types
 import _apache
 
-# a small hack to improve PythonPath performance. This
-# variable stores the last PythonPath in raw (unevaled) form.
-_path = None
+try:
+    import threading
+except:
+    import dummy_threading as threading
+
+# Cache for values of PythonPath that have been seen already.
+_path_cache = {}
+_path_cache_lock = threading.Lock()
 
 class CallBack:
     """
@@ -74,20 +79,19 @@ def ConnectionDispatch(self, conn):
             else:
                 object_str = l[1]
 
-            # add the directory to pythonpath if
-            # not there yet, or pythonpath specified
-
-            if config.has_key("PythonPath"):
-                # we want to do as little evaling as possible,
-                # so we remember the path in un-evaled form and
-                # compare it
-                global _path
-                pathstring = config["PythonPath"]
-                if pathstring != _path:
-                    _path = pathstring
-                    newpath = eval(pathstring)
-                    if sys.path != newpath:
-                        sys.path[:] = newpath
+            # evaluate pythonpath and set sys.path to
+            # resulting value if not already done
+
+            if config.has_key("PythonPath"): 
+                _path_cache_lock.acquire() 
+                try: 
+                    pathstring = config["PythonPath"] 
+                    if not _path_cache.has_key(pathstring): 
+                        newpath = eval(pathstring) 
+                        _path_cache[pathstring] = None 
+                        sys.path[:] = newpath 
+                finally: 
+                    _path_cache_lock.release()
 
             # import module
             module = import_module(module_name,
@@ -153,19 +157,20 @@ def FilterDispatch(self, filter):
                 object_str = l[1]
 
             # add the directory to pythonpath if
-            # not there yet, or pythonpath specified
-
-            if config.has_key("PythonPath"):
-                # we want to do as little evaling as possible,
-                # so we remember the path in un-evaled form and
-                # compare it
-                global _path
-                pathstring = config["PythonPath"]
-                if pathstring != _path:
-                    _path = pathstring
-                    newpath = eval(pathstring)
-                    if sys.path != newpath:
-                        sys.path[:] = newpath
+            # not there yet, or evaluate pythonpath
+            # and set sys.path to resulting value
+            # if not already done
+
+            if config.has_key("PythonPath"): 
+                _path_cache_lock.acquire() 
+                try: 
+                    pathstring = config["PythonPath"] 
+                    if not _path_cache.has_key(pathstring): 
+                        newpath = eval(pathstring) 
+                        _path_cache[pathstring] = None 
+                        sys.path[:] = newpath 
+                finally: 
+                    _path_cache_lock.release()
             else:
                 if filter.dir and (filter.dir not in sys.path):
                     sys.path[:0] = [filter.dir]
@@ -265,18 +270,20 @@ def HandlerDispatch(self, req):
                     object_str = l[1]
 
                 # add the directory to pythonpath if
-                # not there yet, or pythonpath specified
-                if config.has_key("PythonPath"):
-                    # we want to do as little evaling as possible,
-                    # so we remember the path in un-evaled form and
-                    # compare it
-                    global _path
-                    pathstring = config["PythonPath"]
-                    if pathstring != _path:
-                        _path = pathstring
-                        newpath = eval(pathstring)
-                        if sys.path != newpath:
-                            sys.path[:] = newpath
+                # not there yet, or evaluate pythonpath
+                # and set sys.path to resulting value
+                # if not already done
+
+                if config.has_key("PythonPath"): 
+                    _path_cache_lock.acquire() 
+                    try: 
+                        pathstring = config["PythonPath"] 
+                        if not _path_cache.has_key(pathstring): 
+                            newpath = eval(pathstring) 
+                            _path_cache[pathstring] = None 
+                            sys.path[:] = newpath 
+                    finally: 
+                        _path_cache_lock.release()
                 else:
                     dir = hlist.directory
                     if dir and (dir not in sys.path):
@@ -397,7 +404,23 @@ def IncludeDispatch(self, filter, tag, code):
 
     def ImportDispatch(self, name):
 
-        debug = int(main_server.get_config().get("PythonDebug", "0"))
+        config = main_server.get_config()
+
+        debug = int(config.get("PythonDebug", "0"))
+
+        # evaluate pythonpath and set sys.path to
+        # resulting value if not already done
+
+        if config.has_key("PythonPath"): 
+            _path_cache_lock.acquire() 
+            try: 
+                pathstring = config["PythonPath"] 
+                if not _path_cache.has_key(pathstring): 
+                    newpath = eval(pathstring) 
+                    _path_cache[pathstring] = None 
+                    sys.path[:] = newpath 
+            finally: 
+                _path_cache_lock.release()
 
         # split module::function
         l = name.split('::', 1)

src/mod_python.c

@@ -2308,41 +2308,6 @@ static void PythonChildInitHandler(apr_pool_t *p, server_rec *s)
         if (!idata)
             return;
 
-        /* set up PythonPath */
-        ppath = apr_table_get(conf->directives, "PythonPath");
-        if (ppath) {
-
-            PyObject *globals, *locals, *newpath, *sys;
-
-            globals = PyDict_New();
-            locals = PyDict_New();
-
-            sys = PyImport_ImportModuleEx("sys", globals, locals, NULL);
-            if (!sys)
-                goto err;
-
-            PyDict_SetItemString(globals, "sys", sys);
-            newpath = PyRun_String((char *)ppath, Py_eval_input, globals, locals);
-            if (!newpath)
-                goto err;
-
-            if (PyObject_SetAttrString(sys, "path", newpath) == -1)
-                goto err;
-
-            Py_XDECREF(sys);
-            Py_XDECREF(newpath);
-            Py_XDECREF(globals);
-            Py_XDECREF(locals);
-            goto success;
-
-        err:
-            PyErr_Print();
-            fflush(stderr); 
-            release_interpreter();
-            return;
-        }
-
-    success:
         /* 
          * Call into Python to do import.
          * This is the C equivalent of

test/htdocs/tests.py

@@ -884,7 +884,9 @@ def trans(req):
 
 def import_test(req):
 
-    import sys
+    import sys, os
+    directory = os.path.dirname(__file__)
+    assert(sys.path.count(directory) == 1)
     if sys.modules.has_key("dummymodule"):
         if not apache.main_server.get_options().has_key("dummymodule::function"):
             req.log_error("dummymodule::function not executed")