Skip to content

contextlib.redirect_stdout/stderr does not work with pytest-run-parallel #130148

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

Closed
15r10nk opened this issue Feb 15, 2025 · 10 comments
Closed

contextlib.redirect_stdout/stderr does not work with pytest-run-parallel #130148

15r10nk opened this issue Feb 15, 2025 · 10 comments
Labels
stdlib Python modules in the Lib dir topic-free-threading type-feature A feature request or enhancement

Comments

@15r10nk
Copy link
Contributor

15r10nk commented Feb 15, 2025

Bug report

Bug description:

redirect_stdout/stderr does not work if freethreading is enabled.

The following example shows the problem:

# /// script
# dependencies = [
#   "pytest",
#   "pytest-run-parallel"
# ]
# ///


from contextlib import redirect_stdout
from io import StringIO
import time


def test_redirect():
    text=StringIO()
    with redirect_stdout(text):
        print("hello")
        time.sleep(1)
        print("hello")

    assert text.getvalue()=="hello\nhello\n"


if __name__ == "__main__":
    import pytest
    pytest.main(["--parallel-threads=5",__file__])

output:

❯ uv run -p 3.13t bug.py
================================== test session starts ==================================
platform linux -- Python 3.13.1, pytest-8.3.4, pluggy-1.5.0
rootdir: /home/frank/projects/bugs/pytest_redirect
plugins: run-parallel-0.3.1
collected 1 item                                                                        

bug.py F                                                                          [100%]

======================================= FAILURES ========================================
_____________________________________ test_redirect _____________________________________

    def test_redirect():
        text=StringIO()
        with redirect_stdout(text):
            print("hello")
            time.sleep(1)
            print("hello")
    
>       assert text.getvalue()=="hello\nhello\n"
E       AssertionError: assert 'hello\n' == 'hello\nhello\n'
E         
E           hello
E         - hello

bug.py:22: AssertionError
================================ short test summary info ================================
FAILED bug.py::test_redirect - AssertionError: assert 'hello\n' == 'hello\nhello\n'
=================================== 1 failed in 1.04s ===================================

The underlying problem is probably the global sys.stdout/stderr state which is shared between threads.
Is it possible to make these thread local?

CPython versions tested on:

3.13

Operating systems tested on:

No response

@15r10nk 15r10nk added the type-bug An unexpected behavior, bug, or error label Feb 15, 2025
@picnixz picnixz added stdlib Python modules in the Lib dir topic-free-threading interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 15, 2025
@picnixz
Copy link
Member

picnixz commented Feb 15, 2025

The underlying problem is probably the global sys.stdout/stderr state which is shared between threads. Is it possible to make these thread local?

I'm not sure we can do it. AFAIK, those standard streams are at the interpreter state level. I also don't know why we only have a single write and not more. It's as if one thread is not doing anything or something was not caught at all (I would not be surprised if for instance we had more "hello" but I'm surprised we only have one)

cc @ZeroIntensity

@15r10nk
Copy link
Contributor Author

15r10nk commented Feb 15, 2025

The reason for the one hello is that the second thread changes sys.stdout and this thread has more than two hellos. But I get only the first assertion with one hello.

@picnixz
Copy link
Member

picnixz commented Feb 15, 2025

Ah yes I see! ok nvm it was dumb of me.

@colesbury colesbury changed the title contextlib.redirect_stdout/stderr does not work with freethreading contextlib.redirect_stdout/stderr does not work with pytest-run-parallel Feb 15, 2025
@colesbury colesbury added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Feb 15, 2025
@ZeroIntensity
Copy link
Member

Is it possible to make these thread local?

I don't think so, at least without making it a huge breaking change. If we were to make sys.stdout/sys.stderr thread-local, we'd break all programs that currently rely on that for catching data from all threads. In theory, I guess we could have some sort of opt-in system to make stdout and stderr thread-local using some dirty hacks, but it would be a lot of work and I'm not convinced it's worth it. I'd just suggest synchronizing your tests that use redirect_stdout with a lock.

@picnixz picnixz removed the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 15, 2025
@15r10nk
Copy link
Contributor Author

15r10nk commented Feb 15, 2025

I don't think so, at least without making it a huge breaking change.

That is what I expected, but I wanted to ask.

I'd just suggest synchronizing your tests that use redirect_stdout with a lock.

I would have to synchronize all Tests which are using sys.stdout. Every test which is using print could use the wrong output.
I see no way how redirect_stdout can be used in a multi threaded environment.

@devdanzin
Copy link
Contributor

devdanzin commented Feb 15, 2025

Playing with the code here, I stumbled on a segfault in a free-threaded JIT build, that's #130163.

@rhettinger
Copy link
Contributor

FWIW, redirect_stdout was never expected to work across threads, even in a build with a GIL.

Stdout being global state is an inconvenient fact-of-life (much like locale or the various other sys settings).

I would have to synchronize all Tests which are using sys.stdout.

How is it that you're not already getting garbage output with multiple threads calling print() and not using locks? ISTM that is the classic demonstration of why locks or queues a needed for shared resources.

To simplify synchronizing the print tests, consider writing a decorator that adds a shared resource lock to tests that print.

@shared_print
def test_reportgen(self):
    ...

Another possible solution is simpler but more blunt. Save the original stdout before any threads are launched and then shadow the builtin print function at the top of each test file:

from start_up import original_stdout

def print(*args, **kwds):
    "Locking version of print()"
    __builtins__.print(*args, file=original_stdout, **kwds)

Every test which is using print could use the wrong output.
I see no way how redirect_stdout can be used in a multi threaded environment.

More precisely, redirect_stdout creates a race condition if used in a thread that is running concurrently with threads that call print(). Of course, the same would happen with manual redirection, sys.stdout = output_file. And even without redirection, print calls in concurrent threads can step on top of one another. ISTM that printing to stdout is intrinsically in need of synchronization regardless of redirection.

@15r10nk
Copy link
Contributor Author

15r10nk commented Feb 18, 2025

How is it that you're not already getting garbage output with multiple threads calling print() and not using locks?

Because I use it in a pytest plugin and pytest does not use threads. I got this problem when I tried to make my code thread-safe by using pytest-run-parallel.

I assumed that it would be very difficult to change it and that it wouldn't be worth the effort, but I wanted to ask because I didn't know for sure.

I'm capturing the output in my plugin to format the code with black. I will most likely have to rewrite this part, which leads to other problems, but probably easier to solve.

@rhettinger
Copy link
Contributor

I suggest closing this.

@ZeroIntensity ZeroIntensity closed this as not planned Won't fix, can't repro, duplicate, stale Feb 18, 2025
@colesbury
Copy link
Contributor

Changing sys.stdout behavior, such as making it thread-local, doesn't seem like a good idea to me either, but I think we can add functionality to contextlib.redirect_stdout to make it work better with pytest-run-parallel and the similar functionality (gh-127933) we're using to test CPython.

I think it's probably best implemented in an external library, possibly pytest-run-parallel, at least at first.

Here's a rough sketch of the idea:

  • contextlib.redirect_stdout gains an optional argument per_thread=False
  • When called with per_thread=True, contextlib.redirect_stdout wraps the passed stream with a PerThreadStream, and sets sys.stdout to the instance of PerThreadStream
  • PerThreadStream has a slot per-thread, along with a default stream. The PerThreadStream delegates file operations (like write()) the appropriate stream, or to the default stream if no stream is set for the current thread.
  • contextlib.redirect_stdout calls are synchronized with each other, but not necessarily with print() or sys.stdout.write() calls.
  • Calls to contextlib.redirect_stdout(..., per_thread=True) will set the current thread's stream slot, if sys.stdout is already a PerThreadStream
  • The last __exit__ from contextlib.redirect_stdout() removes the PerThreadStream from sys.stdout if all per-thread slots are empty, and restores sys.stdout to the default stream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants