Switched start_blocking_portal() to use daemon threads

agronholm commented 3 months ago

Changes

ThreadPoolExecutor used daemon threads in Python 3.8 and non-daemon threads in 3.9 onwards. But daemon threads are essential for the use case where we want to have a "loitering" blocking portal which will only be shut down via an atexit hook.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that you've fulfilled the following conditions (where applicable):

[X] You've added tests (in tests/) added which would fail without your patch
[ ] You've updated the documentation (in docs/, in case of behavior changes or new features)
[X] You've added a new changelog entry (in docs/versionhistory.rst).

If this is a trivial change, like a typo fix or a code reformatting, then you can ignore these instructions.

Updating the changelog

If there are no entries after the last release, use **UNRELEASED** as the version. If, say, your patch fixes issue #123, the entry should look like this:

* Fix big bad boo-boo in task groups (#123 <https://github.com/agronholm/anyio/issues/123>_; PR by @yourgithubaccount)

If there's no issue linked, just link to your pull request instead by updating the changelog after you've created the PR.

dhirschfeld commented 3 months ago

I can confirm this appears to fix the hang I was observing :+1:

agronholm commented 3 months ago

Thanks for confirming!

dhirschfeld commented 3 months ago

Possibly a random error which might be fixed by retrying the CI?

FAILED tests/streams/test_tls.py::TestTLSStream::test_extra_attributes[trio] - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread Thread-4
<snip>
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host

agronholm commented 3 months ago

Possibly a random error which might be fixed by retrying the CI?

FAILED tests/streams/test_tls.py::TestTLSStream::test_extra_attributes[trio] - pytest.PytestUnhandledThreadExceptionWarning: Exception in thread Thread-4
<snip>
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host

Yup, randomly occurring Windows socket errors are among the hardest to figure out.

gschaffner commented 1 month ago

One possibility to avoid breaking subinterpreters here could be to make this opt-in: def start_blocking_portal(daemon: bool = False).

agronholm commented 1 month ago

One possibility to avoid breaking subinterpreters here could be to make this opt-in: def start_blocking_portal(daemon: bool = False).

I'm not too concerned about subinterpreters. I would've loved to add support for them, but they still don't have a stable Python API, and furthermore, once nogil becomes usable, they might become completely obsolete. I think we'll cross that bridge when we come to it, if ever.

agronholm / anyio