Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSVCRT's spawnve/spawnvpe are not thread safe #50725

Open
jfonseca mannequin opened this issue Jul 13, 2009 · 6 comments
Open

MSVCRT's spawnve/spawnvpe are not thread safe #50725

jfonseca mannequin opened this issue Jul 13, 2009 · 6 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@jfonseca
Copy link
Mannequin

jfonseca mannequin commented Jul 13, 2009

BPO 6476
Nosy @amauryfa, @pitrou
Files
  • spawnve.py: Failing sample with workaround
  • 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 2009-07-13.17:11:07.624>
    labels = ['interpreter-core', 'type-bug']
    title = "MSVCRT's spawnve/spawnvpe are not thread safe"
    updated_at = <Date 2011-07-18.23:31:30.590>
    user = 'https://bugs.python.org/jfonseca'

    bugs.python.org fields:

    activity = <Date 2011-07-18.23:31:30.590>
    actor = 'pitrou'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2009-07-13.17:11:07.624>
    creator = 'jfonseca'
    dependencies = []
    files = ['14495']
    hgrepos = []
    issue_num = 6476
    keywords = []
    message_count = 6.0
    messages = ['90495', '90498', '90503', '140574', '140632', '140634']
    nosy_count = 5.0
    nosy_names = ['amaury.forgeotdarc', 'pitrou', 'jfonseca', 'python-dev', 'steve_hill']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = None
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue6476'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

    @jfonseca
    Copy link
    Mannequin Author

    jfonseca mannequin commented Jul 13, 2009

    MSVCRT's implementation of _spawnve, _spawnvpe, or any version that
    takes an environ as paramater is not thread-safe, because it stores a
    temporary environment string into a global variable.

    _spawnve, _spawnvpe, and friends call a function named _cenvarg which
    concatenate the environment strings into a global variable called
    _aenvptr, which gets free'd and zero'd after each invocation.

    This was causing random build failures in scons when parallel build (-j)
    was enabled.

    The sample application evidences this problem. It also includes a simple
    workaround in python, by acquiring a global lock around os.spawnve, and
    simulating P_WAIT with P_NOWAIT to avoid holding the global lock while
    the child process is running. I believe something along these lines
    should be done for CPython on Windows.

    @jfonseca jfonseca mannequin added type-crash A hard crash of the interpreter, possibly with a core dump interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jul 13, 2009
    @amauryfa
    Copy link
    Member

    Indeed. But shouldn't you use the subprocess module instead?

    @jfonseca
    Copy link
    Mannequin Author

    jfonseca mannequin commented Jul 13, 2009

    Perhaps. I'm not a scons developer -- just an user -- and I don't know
    what versions of python far back in time they want support, but it
    appears it would make sense to use subprocess where available indeed. I
    already I've filled an issue with scons at
    http://scons.tigris.org/issues/show_bug.cgi?id=2449 and linked back to
    this issue and I trust the developers to do whatever they see fit to
    address this problem.

    Instead of just marking this issue as won't fix, shouldn't at least some
    documentation be added, or something sensible like that? In
    http://docs.python.org/library/os.html#process-management os.spawn* are
    not marked as deprecated -- just says that "the subprocess module
    provides more powerful facilities" and is "preferable" --, and it
    certainly doesn't say these functions are not thread safe. It would be a
    pity if other users would have to invest as much time as I did to find
    this race condition (it was rare, and happened in a build farm so we
    couldn't see the windows access violation dialog), and multithreading is
    getting more and more common.

    Also, if the only reason not to fix this is the lack of a patch I don't
    mind in producing one FWIW.

    @briancurtin briancurtin added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Feb 9, 2010
    @stevehill
    Copy link
    Mannequin

    stevehill mannequin commented Jul 18, 2011

    Why has this bug been resolved as "won't fix"? It seems to me that this is a valid issue with something that has not been deprecated, yet it has been decided neither to fix it (despite there being an offer by the originator to submit a patch) nor even to document the deficiency.

    We've been using SCons for the last 3-4 years, during which time we have been plagued by this issue - more so as multi-core machines have become more prevalent. There was a thread on the SCons Users mailing list in March '09, which stopped short of diagnosing the problem and we've lived with it ever since - putting it down to Python being "a bit flaky". I now discover that it is an issue that has been diagnosed two years ago and deliberately left in the implementation of the language. Simply saying "you should use subprocess" is not helpful; SCons at that time was supporting all Python versions back to 2.0 (possibly earlier) so couldn't rely on the subprocess module being present.

    Ideally, it should be worked-around so that these functions can safely be used on all platforms without requiring mutual exclusion in the application. However, it seems to me that, at a bare minimum, it should be documented that these functions are not thread safe under Windows.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 18, 2011

    New changeset 3fa7581f6d46 by Antoine Pitrou in branch '2.7':
    Issue bpo-6476: Document that os.spawnle and os.spawnve are not thread-safe under Windows.
    http://hg.python.org/cpython/rev/3fa7581f6d46

    New changeset a01ba3c32a4b by Antoine Pitrou in branch '3.2':
    Issue bpo-6476: Document that os.spawnle and os.spawnve are not thread-safe under Windows.
    http://hg.python.org/cpython/rev/a01ba3c32a4b

    New changeset aee7c27ea7df by Antoine Pitrou in branch 'default':
    Issue bpo-6476: Document that os.spawnle and os.spawnve are not thread-safe under Windows.
    http://hg.python.org/cpython/rev/aee7c27ea7df

    @pitrou
    Copy link
    Member

    pitrou commented Jul 18, 2011

    I've made the necessary doc changes. I leave it open because I'm not sure what to do with the bugfix request (I agree with the general suggestion to use subprocess instead, though).

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    Status: No status
    Development

    No branches or pull requests

    3 participants