@nikic

I'm not familiar with the fastcgi code, just trying to figure out how all this works... Looking at the current implementation of fcgi_accept_request, if fd < 0, we are first calling on_accept() then accept(), then on_read(), then poll() or select(), then fcgi_read_request(). If fd >= 0, then only fcgi_read_request() is called.

This patch changes to call on_read() also after fcgi_read_request(). However, doesn't that mean that in the case of fd < 0, we will end up calling on_read() twice? Similarly, should on_read be called before fcgi_read_request() or after?

doesn't that mean that in the case of fd < 0, we will end up calling on_read() twice

That can be the case, yes. Current implementation of php-fpm uses this hook just to zero variables, so it's not a problem. But if you think this should fire only once I can dig into it...

Similarly, should on_read be called before fcgi_read_request() or after?

Again, since it's just resetting variables it doesn't matter as far as I understand. But I don't know what the author of that code really wanted for this hook.

@dyeldandi You're right that fpm_request_reading_headers() doesn't seem to do anything particularly critical, so doing two calls won't break things too badly. However it will probably mess with the statistics, as these increments will be done twice.

@nikic
Ah, yes, you are right there. Lemme run a bit of debug to check it and fix it if that's the case.

@nikic

doesn't that mean that in the case of fd < 0, we will end up calling on_read() twice

Yeah, it is being called twice when a new connection arrives, which does mess up the statistics.
I moved this event to just before fcgi_read_request().

Merged via 9814be4, thanks!