Skip to content
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

Fixed server hanging on stop #81

Closed
wants to merge 2 commits into from

Conversation

Zeejfps
Copy link

@Zeejfps Zeejfps commented Nov 29, 2019

Added a listening flag to control the infinite loop in the listening thread.

Inside the loop I am using listener.Pending() to check if any new clients are ready to connect to avoid the blocking Accept call that would otherwise freeze the thread preventing Thread.Join from being used to gracefully terminate the thread.

When the flag is set to false, the while loop is exited. This allows for the listener to be closed on the same thread it was created avoiding the WinSock exception that was being thrown.

Failes the IPv6 test, but I think this is due to the test being incorrect?

@Zeejfps
Copy link
Author

Zeejfps commented Nov 29, 2019

I need to make another push that will fix the Repeated Start Stop Test.

Edit: Done

Made the Active property simply return weather the server is listening.
Added a finally block that will set listening to false.
@miwarnec
Copy link
Collaborator

so just to be clear, even though we call .Interrupt(), it does not end your listener thread?
that shouldn't happen at all

@Zeejfps
Copy link
Author

Zeejfps commented Nov 30, 2019

so just to be clear, even though we call .Interrupt(), it does not end your listener thread?
that shouldn't happen at all

Yes i rand the code from issue #72 and it froze on my windows machine.
Strick firewall settings were in used, but I do not see how this would prevent the server from not being able to terminate.

@miwarnec
Copy link
Collaborator

miwarnec commented Feb 4, 2021

sorry, didn't have time for telepathy in a while.
can you confirm that the issue still exists in the latest version?
did a major rewrite in 2021 :)

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.

2 participants