Guido found a brand new race in tempfile on Linux, due to Linux changing

pid across threads (but in that case, it's still the same process, and so
still sharing the "template" cache in tempfile.py).  Repaired that, and
added a new std test.
On Linux, someone please run that standalone with more files and/or more
threads; e.g.,

    python lib/test/test_threadedtempfile.py -f 1000 -t 10

to run with 10 threads each creating (and deleting) 1000 temp files.

diff --git a/Lib/tempfile.py b/Lib/tempfile.py
index 3e1e08dba21bb1f80549a09f7d550c225591bac3..8ac707d7e0902eaf1b60ed71d95833c2f813183f 100644 (file)
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -67,25 +67,40 @@ def gettempdir():
     return tempdir
 
 
-_pid = None
+# template caches the result of gettempprefix, for speed, when possible.
+# XXX unclear why this isn't "_template"; left it "template" for backward
+# compatibility.
+if os.name == "posix":
+    # We don't try to cache the template on posix:  the pid may change on us
+    # between calls due to a fork, and on Linux the pid changes even for
+    # another thread in the same process.  Since any attempt to keep the
+    # cache in synch would have to call os.getpid() anyway in order to make
+    # sure the pid hasn't changed between calls, a cache wouldn't save any
+    # time.  In addition, a cache is difficult to keep correct with the pid
+    # changing willy-nilly, and earlier attempts proved buggy (races).
+    template = None
+
+# Else the pid never changes, so gettempprefix always returns the same
+# string.
+elif os.name == "nt":
+    template = '~' + `os.getpid()` + '-'
+elif os.name == 'mac':
+    template = 'Python-Tmp-'
+else:
+    template = 'tmp' # XXX might choose a better one
 
 def gettempprefix():
-    """Function to calculate a prefix of the filename to use."""
-    global template, _pid
-    if os.name == 'posix' and _pid and _pid != os.getpid():
-        # Our pid changed; we must have forked -- zap the template
-        template = None
+    """Function to calculate a prefix of the filename to use.
+
+    This incorporates the current process id on systems that support such a
+    notion, so that concurrent processes don't generate the same prefix.
+    """
+
+    global template
     if template is None:
-        if os.name == 'posix':
-            _pid = os.getpid()
-            template = '@' + `_pid` + '.'
-        elif os.name == 'nt':
-            template = '~' + `os.getpid()` + '-'
-        elif os.name == 'mac':
-            template = 'Python-Tmp-'
-        else:
-            template = 'tmp' # XXX might choose a better one
-    return template
+        return '@' + `os.getpid()` + '.'
+    else:
+        return template
 
 
 def mktemp(suffix=""):

diff --git a/Lib/test/output/test_threadedtempfile b/Lib/test/output/test_threadedtempfile
new file mode 100644 (file)
index 0000000..2552877
--- /dev/null
+++ b/Lib/test/output/test_threadedtempfile
@@ -0,0 +1,5 @@
+test_threadedtempfile
+Creating
+Starting
+Reaping
+Done: errors 0 ok 1000

diff --git a/Lib/test/test_threadedtempfile.py b/Lib/test/test_threadedtempfile.py
new file mode 100644 (file)
index 0000000..a5ee12f
--- /dev/null
+++ b/Lib/test/test_threadedtempfile.py
@@ -0,0 +1,85 @@
+"""
+Create and delete FILES_PER_THREAD temp files (via tempfile.TemporaryFile)
+in each of NUM_THREADS threads, recording the number of successes and
+failures.  A failure is a bug in tempfile, and may be due to:
+
++ Trying to create more than one tempfile with the same name.
++ Trying to delete a tempfile that doesn't still exist.
++ Something we've never seen before.
+
+By default, NUM_THREADS == 20 and FILES_PER_THREAD == 50.  This is enough to
+create about 150 failures per run under Win98SE in 2.0, and runs pretty
+quickly. Guido reports needing to boost FILES_PER_THREAD to 500 before
+provoking a 2.0 failure under Linux.  Run the test alone to boost either
+via cmdline switches:
+
+-f  FILES_PER_THREAD (int)
+-t  NUM_THREADS (int)
+"""
+
+NUM_THREADS = 20        # change w/ -t option
+FILES_PER_THREAD = 50   # change w/ -f option
+
+import threading
+from test.test_support import TestFailed
+import StringIO
+from traceback import print_exc
+
+startEvent = threading.Event()
+
+import tempfile
+tempfile.gettempdir() # Do this now, to avoid spurious races later
+
+class TempFileGreedy(threading.Thread):
+    error_count = 0
+    ok_count = 0
+
+    def run(self):
+        self.errors = StringIO.StringIO()
+        startEvent.wait()
+        for i in range(FILES_PER_THREAD):
+            try:
+                f = tempfile.TemporaryFile("w+b")
+                f.close()
+            except:
+                self.error_count += 1
+                print_exc(file=self.errors)
+            else:
+                self.ok_count += 1
+
+def _test():
+    threads = []
+
+    print "Creating"
+    for i in range(NUM_THREADS):
+        t = TempFileGreedy()
+        threads.append(t)
+        t.start()
+
+    print "Starting"
+    startEvent.set()
+
+    print "Reaping"
+    ok = errors = 0
+    for t in threads:
+        t.join()
+        ok += t.ok_count
+        errors += t.error_count
+        if t.error_count:
+            print '%s errors:\n%s' % (t.getName(), t.errors.getvalue())
+
+    msg = "Done: errors %d ok %d" % (errors, ok)
+    print msg
+    if errors:
+        raise TestFailed(msg)
+
+if __name__ == "__main__":
+    import sys, getopt
+    opts, args = getopt.getopt(sys.argv[1:], "t:f:")
+    for o, v in opts:
+        if o == "-f":
+            FILES_PER_THREAD = int(v)
+        elif o == "-t":
+            NUM_THREADS = int(v)
+
+_test()