Skip to content

Conversation

@15r10nk
Copy link
Owner

@15r10nk 15r10nk commented Feb 6, 2025

solves #192

@15r10nk 15r10nk force-pushed the thread-safe branch 14 times, most recently from b4d8d74 to 47e4225 Compare February 9, 2025 22:10
@davidhewitt
Copy link

Thanks for working on this! If you have specific things you'd like help on, myself or @ngoldbaum may be of use :)

@15r10nk
Copy link
Owner Author

15r10nk commented Feb 11, 2025

I think the two remaining problems are the following:

@davidhewitt
Copy link

I guess for black, you might be able to just put a lock around that? Sadly warnings.catch_warnings is also not thread safe (I think there might be new APIs considered for 3.14).

Pydantic support is on the way, follow pydantic/pydantic#10483 - probably will be ready in ~1 month.

@15r10nk
Copy link
Owner Author

15r10nk commented Feb 14, 2025

I guess for black, you might be able to just put a lock around that?

This does not work. The problem is that another thread is writing something to sys.stdout at the same time the CliRunner is mocking it to capture the output of black.
I will have to use the internal api functions here. I just hope that I don't have to re-implement the config option parsing of black.

The following code shows the problem with stdout.

# /// 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__])

contextlib.redirect_stdout is also not thread save for the same reason. It is modifying global state. I don't know if stdin/stdout can be fixed in the same way.

@ngoldbaum
Copy link

Maybe open a CPython bug? It seems reasonable to me for python to add locking for global state in the C standard library.

@15r10nk
Copy link
Owner Author

15r10nk commented Feb 15, 2025

I created an issue python/cpython#130148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants