Support adding/removing socket listeners on already running servers #159
Conversation
|
This needs to be thread safe. |
jooooel
force-pushed
the
jooooel/add_socket_support
branch
from
32530ac to
9543a57
Compare
- Make it possible to add and remove socket listeners to already running servers. Relies on the existing methods, just adds and removes "RunningListeners" as needed. Backed by Dictionary instead of List to simplify lookups - Handle concurrency by using a SemaphorSlim, similar to how it's done in Kestrel - Added equivalent methods to ServerHostedService, since the Server itself isn't accessible. - Added some test coverage for the new behavior.
jooooel
force-pushed
the
jooooel/add_socket_support
branch
from
9543a57 to
2b6e433
Compare
|
@davidfowl Oh, you're right! Didn't think about that :) I don't often come across areas where I have to think about thread safety in my daily work, but I've given it a go here. Let me know if there are any gotchas or something special to look out for... And I'm not really sure how to solve this the best way. I started out with a ConcurrentDictionary for the RunningListeners, but I soon realised that wasn’t enough. So I went for a simple locking instead. This way, the server will have to wait until any Add/Remove operation has finished, and vice versa. I took a look at how Kestrel handles similar cases, and was inspired by that. I think I need a bit of help reasoning about the heartbeat timer, if there are possible concurrency issues there as well. I guess the risk is that a listener currently/already being stopped receives a call to |
|
But basically all I did was to await a |
|
And by the way. Do with this what you will 😊 I’m not expecting you to spend any of your free time on me and my tinkering |
I'm working on adding support for starting and stopping individual services in Project Tye, without restarting Tye entirely. One thing I'm missing is support for adding and removing socket listeners to an already running BedrockFramework server (needed by the
ProxyServicein Tye).I've taken a shot at implementing that here, with the hope that it could get released into a new nuget version and be used in my fork of Tye.
I took the liberty to add some tests for this new behaviour. Maybe it's arguable if they are unit or integration tests (because of the use of sockets etc), but they seem to run fine.
Hope you're open to this change, and please let me know if you'd like me to change it it any way 😊