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?

Also a more general question: Can someone tell me why the poll/select is only done for the fd<0 case? Why is this not applicable to the keepalive case?

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.

Also a more general question: Can someone tell me why the poll/select is only done for the fd<0 case? Why is this not applicable to the keepalive case?

Erm, maybe because in keepalive case the connection is already there and no need to add it to select() set?

I'm probably horribly misunderstanding something here... But isn't the connection also already there is in the case where initially fd was <0? After all, the poll/select is run after the accept(), not before it.

Ahhh, I get it know. The poll/select is there to add a 5 second timeout for the initial read after accept. Of course this should not be there for a keepalive connection.