From 9fadfb0d1dbf1b42efbe1374ea55f99b57b76a3f Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Sat, 13 Jan 2001 03:04:02 +0000 Subject: 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. --- Lib/tempfile.py | 47 ++++++++++++------- Lib/test/output/test_threadedtempfile | 5 +++ Lib/test/test_threadedtempfile.py | 85 +++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 16 deletions(-) create mode 100644 Lib/test/output/test_threadedtempfile create mode 100644 Lib/test/test_threadedtempfile.py diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 3e1e08d..8ac707d 100644 --- 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 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 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() -- cgit v0.12