simple-http icon indicating copy to clipboard operation
simple-http copied to clipboard

Thread Safety & Static Route handling

Open JKamsker opened this issue 8 years ago • 2 comments

Hey, I've noticed that the Method collection is a simple list which is NOT thread safe. Using it in a multithreaded application (==Task.Run) can result in weird behaviour which could potentially destroy the process. I suggest moving to a collection from System.Collections.Concurrent or using an access lock object and locking it with "lock(object){}". You should pay attention to deadlocks.

Another thing which i noticed is that the Route class is static which isnt optimal. There are 2 reasons which came to my mind:

  1. Testability. You will have a hard time Unit Testing your application.
  2. Multiple server instances with different behaviours are not possible with the current approach.

I hope you find those suggestion helpful

JKamsker avatar Feb 22 '18 16:02 JKamsker

Hi Jonas,

thanks for the comments.

I absolutely agree on the first comment, thank you for that.

However, I do not know how to address the second one efficiently. What I mean to say is, I can do that, but then I would be heading towards NancyFX framework design. I intended to create a bare-bone library, where the route methods can be scattered in multiple classes. If you have an idea how to do that efficiently, please do share.

Best, Darko

2018-02-22 17:48 GMT+01:00 Jonas Kamsker [email protected]:

Hey, I've noticed that the Method collection is a simple list which is NOT thread safe. Using it in a multithreaded application (==Task.Run) can result in weird behaviour which could potentially destroy the process. I suggest moving to a collection from System.Collections.Concurrent or using an access lock object and locking it with "lock(object){}". You should pay attention to deadlocks.

Another thing which i noticed is that the Route class is static which isnt optimal. There are 2 reasons which came to my mind:

  1. Testability. You will have a hard time Unit Testing your application.
  2. Multiple server instances with different behaviours are not possible with the current approach.

I hope you find those suggestion helpful

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dajuric/simple-http/issues/1, or mute the thread https://github.com/notifications/unsubscribe-auth/AEMQJx14IboDBjiYUk8fPs2GheElCUL1ks5tXZp7gaJpZM4SPoZm .

dajuric avatar Feb 25 '18 11:02 dajuric

I prefer normal instance initialization:

var server = new HttpServer(8080);
server.Route(...);
server.Start();

KISS: if anyone wants to have static init scattered among classes, then she just may do a singleton from HttpServer.

xmedeko avatar Oct 12 '18 09:10 xmedeko