olliNiinivaara / GuildenStern

Modular multithreading HTTP/1.1 + WebSocket server framework
MIT License
80 stars 7 forks source link

Proper fix for #2 (Occasional connection reset) #4

Closed aboisvert closed 3 years ago

aboisvert commented 3 years ago

Note: This (draft) pull request doesn't line up to current master branch on https://github.com/olliNiinivaara/GuildenStern but I figured I'd share it early. I hadn't updated with the recent changes. I can work on applying the fixes to master

Overall description of the fixes:

As it turns out, Selector.unregister() clears the selector's key under the hood. This created a situation where file descriptor 0 (normally the stdin) would get closed. FD 0 would quickly be reused for new incoming connection's socket. And .. eventually this socket would be arbitrarily closed, possibly by another thread calling unregister(), e.g. via socketClose(). So this caused all sorts of unpredictable trouble when connections were closed by clients, or when connections were established and supplied invalid payload.

Another bug fixed by this commit is a potential double-close() on file descriptors. This is dangerous because when a FD is closed, it becomes available for allocation again, so closing it twice may close another thread's active socket.

(See contextual comments in the code)