Python 3.5.2 crashers (from PyPy)

[arigo]

BPO	28883
Nosy	@brettcannon, @arigo, @vstinner, @serhiy-storchaka

^{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 2016-12-06.11:49:47.741>
labels = ['type-crash']
title = 'Python 3.5.2 crashers (from PyPy)'
updated_at = <Date 2018-02-25.12:58:11.810>
user = 'https://github.com/arigo'

bugs.python.org fields:

activity = <Date 2018-02-25.12:58:11.810>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2016-12-06.11:49:47.741>
creator = 'arigo'
dependencies = []
files = []
hgrepos = []
issue_num = 28883
keywords = []
message_count = 9.0
messages = ['282518', '282519', '282520', '282521', '282522', '282523', '282525', '282564', '312809']
nosy_count = 4.0
nosy_names = ['brett.cannon', 'arigo', 'vstinner', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue28883'
versions = ['Python 3.5']

Linked PRs

• gh-73069: fix race conditions in os.scandir and associated direntry #131642

[arigo]

As discussed on python-dev, I am creating omnibus issues from the lists of crashers, of wrong-according-to-the-docs, and of strange-behavior-only issues that I found while developing Python 3.5.2 support for PyPy. These occur with CPython 3.5.2 but most of them are likely still here in trunk.

This is the issue containing the crashers.

[arigo]

os.scandir() returns an iterable object that should not be used
from multiple threads. Doing so can e.g. cause one thread to
close the dirp while another thread is still using it. This is
likely to crash. Similarly, the test for (!iterator->dirp) at
the start of ScandirIterator_iternext() is only done once even
if the following loop runs two or three times because of "." or
".." entries.

[arigo]

(C2) os.scandir() direntry objects should not have stat() called from two
threads concurrently. It will make two stat objects and leak one of
them.

[arigo]

(C3) _PyGen_yf() checks the opcode at [f_lasti + 1], which is the next
opcode that will run when we resume the generator: either it is the
opcode following the YIELD, or it is exactly YIELD_FROM. It is not
possible at the moment to write Python code that compiles to a YIELD
immediately followed by YIELD_FROM, so by chance the two cases are
correctly distinguished. *However,* the discussion so far assumes
that the generator is not currently running. If it is (which probably
doesn't occur in reasonable Python code but can be constructed
manually), then this checks for example the byte/word that describes
the argument of the currently running opcode. If we're very unlucky
this byte has the value 72, which is YIELD_FROM. Total nonsense and
crashes follow.

[arigo]

(C4) faulthandler: register(): the signal handler, faulthandler_user(),
changes errno in faulthandler_dump_traceback() but fails to restore it
if chain=False. This can rarely cause random nonsense in the main
program.

[arigo]

(C5) setting f_lineno didn't evolve when the rest of the bytecodes evolved,
which means it is not safe any more::

    import sys

    def f():
        try:
            raise ValueError    # line 5
        except ValueError:
            print(42)           # line 7

    def my_trace(*args):
        print(args)
        if args[1] == 'line':
            f = args[0]
            if f.f_lineno == 5:
                f.f_lineno = 7
        return my_trace

    sys.settrace(my_trace)
    f()
    sys.settrace(None)

[arigo]

(C6) I didn't try, but it seems that typeobject.c:mro_internal() is prone
to a refcount crash. It does this::

     old_mro = type->tp_mro;
     ...mro_invoke()...  /* might cause reentrance */
     type->tp_mro = new_mro;
     ...
     Py_XDECREF(old_mro);

This last XDECREF drops the reference held by the previous value of
type->tp_mro after we changed it. But type->tp_mro might have
changed because of mro_invoke(), which calls pure Python code. If it
did change, then old_mro is no longer the old value of
type->tp_mro. The wrong object gets decrefed.

[brettcannon]

Victor for C4.

[serhiy-storchaka]

C5 is fixed in 3.8 by bpo-17611.

[iritkatriel]

I thiink C6 was fixed in #66924.

[iritkatriel]

@brandtbucher - is there any chance that C3(at #73069 (comment)) is still relevant?

[duaneg]

@brandtbucher - is there any chance that C3(at #73069 (comment)) is still relevant?

It looks like c3 has been fixed by gh-111354, although I don't understand that code well enough to be 100% sure. Assuming so, and if my fixes for c1 & c2 are acceptable, once they are merged all the issues will have been fixed and this can be closed.

[duaneg]

Reproducers for c1 & c2:

c1.py

import os
import threading

N=8
ITERATIONS=1000

def scan(iter, barrier):
    barrier.wait()
    for entry in iter:
        pass

for _ in range(ITERATIONS):
    barrier = threading.Barrier(N)
    iter = os.scandir()

    threads = [threading.Thread(target=scan, args=(iter, barrier)) for _ in range(N)]
    for t in threads:
        t.start()
    for t in threads:
        t.join()

c2.py

# Shows leaks with --with-address-sanitizer on Linux, unsure how to verify on Windows

import os
import threading

N=8
ITERATIONS=100

def stat(entry, barrier):
    barrier.wait()
    entry.stat()

for _ in range(ITERATIONS):
    barrier = threading.Barrier(N)
    iter = os.scandir()
    for entry in iter:
        threads = [threading.Thread(target=stat, args=(entry, barrier)) for _ in range(N)]
        for t in threads:
            t.start()
        for t in threads:
            t.join()

Note these reproduce the problem with N=2 and ITERATIONS=2 on my machine, but I bumped the numbers up to stress-test under TSAN.