Thread Safety & Static Route handling

[JKamsker]

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

[dajuric]

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 <notifications@github.com>:

…

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 <#1>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEMQJx14IboDBjiYUk8fPs2GheElCUL1ks5tXZp7gaJpZM4SPoZm> .

[xmedeko]

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.