search

miyagawa / Starman

Starman is a high-performance preforking Perl PSGI web server
http://search.cpan.org/dist/Starman
Other
287 stars 84 forks source link

Treat ECONNRESET like EPIPE (i.e. ignore), not as a fatal error. #114

Closed timbunce closed 9 years ago

timbunce commented 9 years ago

If the client closed the connection before reading all the data, or there's a network failure that results in the connection being reset, we shouldn't treat that as a fatal error. It's much more like EPIPE so now we treat it that way.

timbunce commented 9 years ago

We encountered this issue due to a health-check that only read the headers of the response (sent by the first _syswrite() call) and then closed the connection without reading the body (sent by a second _syswrite() call).

So there was a race-hazard: From time-to-time the OS would notice the socket closing before the second syswrite and then we'd get a "write error: Connection reset by peer" error.

ap commented 9 years ago

Nice – this has bugged me before, but never enough yet to get me to investigate.

Is there a reason you didn’t put it in the same statement as in return if $! == EPIPE or $! == ECONNRESET?

miyagawa commented 9 years ago

Looks good. Is there a way to locally test this behavior to get it verified?

timbunce commented 9 years ago

@ap: no reason not to use return if $! == EPIPE or $! == ECONNRESET; - I was just keeping the diff as trivial as possible. @miyagawa: not easy to test locally, but tested in staging and will be deployed to production today :)

miyagawa commented 9 years ago

Well if it's not as easy as a unit test, a contrived example to reproduce the issue would be more than appreciated.

timbunce commented 9 years ago

Thanks. Sorry for not getting around to making a test for it. We've had it in production for weeks though.