-
Notifications
You must be signed in to change notification settings - Fork 564
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
Exception in socket acceptor thread crashing the whole application #392
Comments
Hi, I am finding a similar situation where a socket exception is bringing down the entire application. Any recommended resolution for this ? |
Hey. My team and I are also facing this issue - We can't catch the Port already being used as no Error gets thrown up to us and so this causes our whole application to crash at some point soon after. |
@gbirchmeier just wanting to float this to the top of the pile |
Commening here only to acknowledge this issue. I'm not sure which of @RemiGaudin 's solutions would be better. I agree that his option (1) doesn't seem disruptive-- in fact, it cuts out a |
Is there an easy-ish way to force the _tcpListener.Start exception and cause this issue to occur? |
This issue looks similar than #257 but this one is on the acceptor side, more precisely in the ThreadedSocketReactor.Run() function.
If any exception occurs while calling tcpListener_.Start() in ThreadedSocketReactor.Run(), then the whole application is crashing as there is no try/catch and the function runs on a dedicated thread. Typically an exception would occur here when the port is already in use by another application, thus the issue is easy to reproduce.
I would be glad to provide a fix via a PR but before that I would like some advice about the best way to fix it. In my opinion this is not enough to just silently catch tcpListener_.Start(), we should also notify the parent application in some way otherwise it won't know that the acceptor did not start successfully. With that in mind I see two different approaches:
There's certainly other ways to fix that so I'm open to any ideas.
The text was updated successfully, but these errors were encountered: