fix one or two bugs in trace.py

[zooko]

BPO	766910
Nosy	@abalkin, @devdanzin
Files	makedir.patch trace-dir.patch.txt trace_target.py traced_module.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2003-07-07.01:46:30.000>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = 'fix one or two bugs in trace.py'
updated_at = <Date 2021-12-10.20:38:21.739>
user = 'https://bugs.python.org/zooko'

bugs.python.org fields:

activity = <Date 2021-12-10.20:38:21.739>
actor = 'ajaksu2'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2003-07-07.01:46:30.000>
creator = 'zooko'
dependencies = []
files = ['5452', '8480', '19933', '19934']
hgrepos = []
issue_num = 766910
keywords = ['patch']
message_count = 14.0
messages = ['44232', '55404', '56196', '56197', '58923', '65432', '114249', '123005', '123319', '126113', '126154', '126155', '190040', '190088']
nosy_count = 5.0
nosy_names = ['zooko', 'belopolsky', 'ajaksu2', 'ijmorlan', 'eli.bendersky']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue766910'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

Linked PRs

• gh-38807: fix race condition in Lib/trace.py #110143
• [3.12] gh-38807: Fix race condition in Lib/trace.py (GH-110143) #114206
• [3.11] gh-38807: Fix race condition in Lib/trace.py (GH-110143) #114207

[zooko]

There is one bug and one maybe-bug in creation of the
coverage file directory in trace.py.

The maybe-bug is that it doesn't attempt to create the
directory if it got the directory name from the name of
the .py file (instead of getting the directory name
from the --coverdir command line option).

Normally the directory will already exist, but if you
are writing out coverage files from a stored report
(or, I suppose, if someone deleted the directory after
the modules were loaded but before you wrote out the
report), then it won't. This patch makes it so that it
always attempts to create the directory before
attempting to write files into it. The patch also adds
a statement to that effect to the "--help" usage info.

The other bug is that it attempts to create a directory
with:

                if not os.path.exists(dir):
                    os.makedirs(dir)

, which is a race condition that will very rarely raise
a spurious exception ("File exists:") if two threads or
processes execute it at the same time.

The fix provided in this patch is to copy from my
pyutil project [1] a utility function that works around
this race condition and invoke that function. On
request I'll provide a slimmed-down version of that
function, since we don't use all of its options in
trace.py.

This patch hasn't been tested at ALL. In fact, I
haven't tested trace.py in general since before it
moved from Tools/scripts to Lib/. As this patch ought
to go into Python 2.3, it ought to be tested, and I
promise to do so soon.

[1] http://sf.net/projects/pyutil

[smontanaro]

Zooko, is this patch still necessary?

[zooko]

Hi! Sorry it took me so long to look at this. I just checked the
source in current trunk, and the relevant code is the same so this
patch is still useful. (See the initial post for details.)

Here is an updated version of the patch which simply removes some
dead code and updates a URL:

regards,

Zooko

diff -rN -u old-up/setuptools-0.6c7/ez_setup.py new-up/ 
setuptools-0.6c7/ez_setup.py
--- old-up/setuptools-0.6c7/ez_setup.py	2007-09-28 16:41:24.000000000  
-0600
+++ new-up/setuptools-0.6c7/ez_setup.py	2007-09-28 16:41:25.000000000  
-0600
@@ -1,4 +1,4 @@
-#!python
+#!/usr/bin/env python
"""Bootstrap setuptools installation
If you want to use setuptools in your package's setup.py, just  
include this
@@ -13,7 +13,7 @@
This file can also be run as a script to install or upgrade setuptools.
"""
-import sys
+import os, re, subprocess, sys
DEFAULT_VERSION = "0.6c7"
DEFAULT_URL     = "http://pypi.python.org/packages/%s/s/setuptools/"  
% sys.version[:3]
@@ -44,8 +44,6 @@
      'setuptools-0.6c6-py2.5.egg': 'b2f8a7520709a5b34f80946de5f02f53',
}
-import sys, os
-
def _validate_md5(egg_name, data):
      if egg_name in md5_data:
          from md5 import md5
@@ -58,6 +56,42 @@
              sys.exit(2)
      return data
+# The following code to parse versions is copied from  
pkg_resources.py so that
+# we can parse versions without importing that module.
+component_re = re.compile(r'(\d+ | [a-z]+ | \.| -)', re.VERBOSE)
+replace = {'pre':'c',  
'preview':'c','-':'final-','rc':'c','dev':'@'}.get
+
+def _parse_version_parts(s):
+    for part in component_re.split(s):
+        part = replace(part,part)
+        if not part or part=='.':
+            continue
+        if part[:1] in '0123456789':
+            yield part.zfill(8)    # pad for numeric comparison
+        else:
+            yield '*'+part
+
+    yield '*final'  # ensure that alpha/beta/candidate are before final
+
+def parse_version(s):
+    parts = []
+    for part in _parse_version_parts(s.lower()):
+        if part.startswith('*'):
+            if part<'*final':   # remove '-' before a prerelease tag
+                while parts and parts[-1]=='*final-': parts.pop()
+            # remove trailing zeros from each series of numeric parts
+            while parts and parts[-1]=='00000000':
+                parts.pop()
+        parts.append(part)
+    return tuple(parts)
+
+def setuptools_is_new_enough(required_version):
+    """Return True if setuptools is already installed and has a version
+    number >= required_version."""
+    sub = subprocess.Popen([sys.executable, "-c", "import  
setuptools;print setuptools.__version__"], stdout=subprocess.PIPE)
+    verstr = sub.stdout.read().strip()
+    ver = parse_version(verstr)
+    return ver and ver >= parse_version(required_version)
def use_setuptools(
      version=DEFAULT_VERSION, download_base=DEFAULT_URL,  
to_dir=os.curdir,
@@ -74,32 +108,11 @@
      this routine will print a message to ``sys.stderr`` and raise  
SystemExit in
      an attempt to abort the calling script.
      """
-    try:
-        import setuptools
-        if setuptools.__version__ == '0.0.1':
-            print >>sys.stderr, (
-            "You have an obsolete version of setuptools installed.   
Please\n"
-            "remove it from your system entirely before rerunning  
this script."
-            )
-            sys.exit(2)
-    except ImportError:
+    if not setuptools_is_new_enough(version):
          egg = download_setuptools(version, download_base, to_dir,  
download_delay)
          sys.path.insert(0, egg)
          import setuptools; setuptools.bootstrap_install_from = egg
-    import pkg_resources
-    try:
-        pkg_resources.require("setuptools>="+version)
-
-    except pkg_resources.VersionConflict, e:
-        # XXX could we install in a subprocess here?
-        print >>sys.stderr, (
-            "The required version of setuptools (>=%s) is not  
available, and\n"
-            "can't be installed while this script is running. Please  
install\n"
-            " a more recent version first.\n\n(Currently using %r)"
-        ) % (version, e.args[0])
-        sys.exit(2)
-
def download_setuptools(
      version=DEFAULT_VERSION, download_base=DEFAULT_URL,  
to_dir=os.curdir,
      delay = 15
@@ -150,9 +163,14 @@
def main(argv, version=DEFAULT_VERSION):
      """Install or upgrade setuptools and EasyInstall"""
-    try:
-        import setuptools
-    except ImportError:
+    if setuptools_is_new_enough(version):
+        if argv:
+            from setuptools.command.easy_install import main
+            main(argv)
+        else:
+            print "Setuptools version",version,"or greater has been  
installed."
+            print '(Run "ez_setup.py -U setuptools" to reinstall or  
upgrade.)'
+    else:
          egg = None
          try:
              egg = download_setuptools(version, delay=0)
@@ -162,31 +180,6 @@
          finally:
              if egg and os.path.exists(egg):
                  os.unlink(egg)
-    else:
-        if setuptools.__version__ == '0.0.1':
-            # tell the user to uninstall obsolete version
-            use_setuptools(version)
-
-    req = "setuptools>="+version
-    import pkg_resources
-    try:
-        pkg_resources.require(req)
-    except pkg_resources.VersionConflict:
-        try:
-            from setuptools.command.easy_install import main
-        except ImportError:
-            from easy_install import main
-        main(list(argv)+[download_setuptools(delay=0)])
-        sys.exit(0) # try to force an exit
-    else:
-        if argv:
-            from setuptools.command.easy_install import main
-            main(argv)
-        else:
-            print "Setuptools version",version,"or greater has been  
installed."
-            print '(Run "ez_setup.py -U setuptools" to reinstall or  
upgrade.)'
-
-
def update_md5(filenames):
      """Update our built-in md5 registry"""

[zooko]

Here is an updated version of the patch which simply removes some
dead code and updates a URL:

regards,

Zooko

diff -rN -u old-up/setuptools-0.6c7/ez_setup.py new-up/
setuptools-0.6c7/ez_setup.py
--- old-up/setuptools-0.6c7/ez_setup.py 2007-09-28
16:41:24.000000000 -0600
+++ new-up/setuptools-0.6c7/ez_setup.py 2007-09-28
16:41:25.000000000 -0600

Oops, the in-lined patch contents were a different patch entirely,
but the attached patch file was correct.

Just for completeness, here is the correct in-lined patch contents:

Index: Lib/trace.py
===================================================================

--- Lib/trace.py	(revision 58282)
+++ Lib/trace.py	(working copy)
@@ -85,7 +85,12 @@
-r, --report          Generate a report from a counts file; do not  
execute
                        any code.  `--file' must specify the results  
file to
                        read, which must have been created in a  
previous run
-                      with `--count --file=FILE'.
+                      with `--count --file=FILE'.  If --coverdir is not
+                      specified, the .cover files will be written  
into the
+                      directory that the modules were in when the  
report was
+                      generated.  Whether or not --coverdir is  
specified,
+                      --report will always create the cover file  
directory if
+                      necessary.
Modifiers:
-f, --file=<file>     File to accumulate counts over several runs.
@@ -197,6 +202,33 @@
      filename, ext = os.path.splitext(base)
      return filename
+# The following function is copied from the fileutil module from the  
pyutil
+# project:
+# http://pypi.python.org/pypi/pyutil
+# We use this function instead of os.makedirs() so that we don't get a
+# spurious exception when someone else creates the directory at the  
same
+# moment we do.  (For example, another thread or process that is  
also running
+# trace.)
+def make_dirs(dirname, mode=0777):
+    """
+    A threadsafe and idempotent version of os.makedirs().  If the  
dir already
+    exists, do nothing and return without raising an exception.  If  
this call
+    creates the dir, return without raising an exception.  If there  
is an
+    error that prevents creation or if the directory gets deleted after
+    make_dirs() creates it and before make_dirs() checks that it  
exists, raise
+    an exception.
+    """
+    tx = None
+    try:
+        os.makedirs(dirname, mode)
+    except OSError, x:
+        tx = x
+
+    if not os.path.isdir(dirname):
+        if tx:
+            raise tx
+        raise exceptions.IOError, "unknown error prevented creation  
of directory, or deleted the directory immediately after creation: % 
s" % dirname # careful not to construct an IOError with a 2-tuple, as  
that has a special meaning...
+
class CoverageResults:
      def __init__(self, counts=None, calledfuncs=None, infile=None,
                   callers=None, outfile=None):
@@ -290,15 +322,15 @@
              if filename.endswith((".pyc", ".pyo")):
                  filename = filename[:-1]
-            if coverdir is None:
+            if coverdir is not None:
+                dir = coverdir
+                modulename = fullmodname(filename)
+            else:
                  dir = os.path.dirname(os.path.abspath(filename))
                  modulename = modname(filename)
-            else:
-                dir = coverdir
-                if not os.path.exists(dir):
-                    os.makedirs(dir)
-                modulename = fullmodname(filename)
+            make_dirs(dir)
+
              # If desired, get a list of the line numbers which  
represent
              # executable content (returned as a dict for better  
lookup speed)
              if show_missing:

[ijmorlan]

I would suggest that the need to create directories that may already
exist (really "ensure existence of" directories) is not exclusive to
trace.py. I am suggesting this be added as an option to os.mkdir and
os.makedirs. See bpo-1675.

[smontanaro]

unassigning

[BreamoreBoy]

Is there any interest in this issue?