[natefinch]

Stream.wait_closed() is currently a no-op, so callers of open_udp_connection can't wait for the spawned UDP client task to finish. For example, it may still log after the Python side has torn down.

UDP client streams now hold a watch::Receiver whose sender is dropped when the task finishes, and wait_closed waits for that. Streams from servers are unchanged.

mitmproxy needs to await this after closing a stream. That change is in mitmproxy/mitmproxy#8456.

[mhils]

I'm curious: What's the use case for waiting for the underlying UDP task to finish? I mean I understand the general appeal of wait_closed not being a no-op, but why specifically (only) UDP? :)

[natefinch]

Good question! The concrete case is a segfault at shutdown. The task spawned by open_udp_connection can still be running when the process exits, and if it logs (e.g. "UDP client errored"), pyo3-log grabs the GIL during finalization and crashes. From a macOS crash report, the main thread is in exit() / OPENSSL_cleanup while the UDP task is in:

PyEval_RestoreThread                      <- SIGSEGV
PyGILState_Ensure
pyo3::internal::state::AttachGuard::do_attach_unchecked
<pyo3_log::Logger as log::Log>::log
mitmproxy_rs::udp_client::open_udp_connection::{{closure}}

It's UDP-only because that's the only Stream backed by its own task: server-side streams are covered by the server's wait_closed (and #329), and TCP client connections are plain asyncio streams. With this and mitmproxy/mitmproxy#8456, mitmproxy waits for the task before dropping the connection, so it can't outlive the interpreter.

[mhils]

Ah, so this is vorner/pyo3-log#30 again. I really wonder if it's worth fixing this whack-a-mole, or if we should rather patch pyo3-log to not be a pain in the butt anymore (there's vorner/pyo3-log#78 for instance). Do you have any thoughts on that?

[natefinch]

Sorry, missed this until now. I agree that there should be a fix to the late logging issue independent of this fix, but this fix is still an improvement to the code on its own, so that the wait actually waits, to avoid other possible issues where shutdown isn't clean.