Skip to content

Fixes termination issues regarding #4 #5

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

donmahallem
Copy link

One of the main issues was a timing issue between initializing the connection and already accepting send requests. Antoher option would be to block accepting send requests until the connection has status connected.

I am not yet sure if this the final solution but Termination via KeyboardInterupt looks reliable for now.

in some cases this blocked (probably race condition with _ready) it blocked the lock in send
@NicoBiernat
Copy link
Member

Thank you very much for the pull request!
On which OS did you test this?
We figured out that the KeyboardInterrupt actually preemptively interrupts the program correctly on Linux but does not do so on Windows.
We have to test this on both OSs (and also macOS).
So this PR might not fully solve the KeyboardInterrupt issue but the addition of a connection-state seems reasonable nonetheless from what I just quickly looked at.
@Gedusim Could you take a look at this?

@donmahallem
Copy link
Author

donmahallem commented Apr 16, 2025

Currently tested on Windows(where I had the issues atleast) and WSL with Python 3.12. For me it works reliable for now.

One might add an additional lock for the connection_state (I wouldn't reuse the existing one even with RLock) but I considered it not necessary as it is publicly read only.

While debugging this I found some unrelated stuff that could be improved and I will follow up with PRs when I get to them.

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.

(Keyboard)Interupt doesn't get propagated correctly
3 participants