gh-90369: change test server threads to joinable#135560
gh-90369: change test server threads to joinable#135560duaneg wants to merge 1 commit intopython:mainfrom
Conversation
These were made daemons in an otherwise unrelated commit. The test code that joins them was not deleted and still works, so they can just be made joinable again.
|
This does not impact users in any way, so IIUC it doesn't need a NEWS entry. |
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
From what I understand, daemon threads are still useful even when joining, because they allow you to close them with a single CTRL+C. With a non-daemon thread, you have to interrupt it twice: once to stop the join in Python, and once more to stop the join by the interpreter.
What's the rationale here?
It prevents a potential crash if a broken test exits uncleanly and doesn't join, as per the code in the original issue, but this is all very theoretical, for sure. As I said in the issue: I would not normally consider this sort of change worthwhile, but it looked like the threads may have been made daemons by mistake1, it is a trivial change, and I was under the impression we2 prefer joinable threads, all being equal. If you or others don't think it is a good idea I am happy to withdraw it. Note that IMO #90369 should be closed regardless: it is doing things it shouldn't with internal test code. 1 Although, given your point about single CTRL-Cs, possibly it a convenience/QoL tweak by a dev who was frequently interrupting tests during development? |
Probably. IIRC, we do something similar for the |
|
As I said, I'm perfectly happy with that outcome. I'll leave it for a while in case anyone else wants to weigh in, especially the dev who changed them to daemon threads originally, but otherwise I'll close it myself if no-one else does first. |
|
Thanks very much for your review and insights, @ZeroIntensity. I'll close this PR now. The associated issue can also be closed, as discussed. |
2 participants
These were made daemons in an otherwise unrelated commit. The test code that joins them was not deleted and still works, so they can just be made joinable again.