I looked over the code, and I believe you seriously broke things in
r37838, without improving anything related to the issue.
The default timeout for reading from an APR (or Unix) socket is
infinity. During pipelining, we set the timeout to 0 for some write
opreations so that we can try them without blocking. If APR had
separate timeouts for read and write, I would have only set the write
timeout in the first place, and that code in sock_read_cb would be
unnecessary. The comment could perhaps have explained this better, but
I'm not sure if it could have explained it well enough to dispel your
assumptions about what the code was doing.
What you did was remove the code which reverts back to blocking, leaving
the timeout at 0 in certain situations. I don't know exactly what
symptoms this would cause, but I imagine they're nothing good.
As a matter of history, I've not been impressed with application-level
hardcoded timeouts, so I didn't put any into ra_svn. It's not the case,
as you said in #3347, that I explicitly removed some default timeout
present in APR or the operating system; those layers don't have default
timeouts.
What I would recommend to address #3347 is setting the SO_KEEPALIVE flag
on the socket, which will cause the underlying TCP layer to use
keepalive packets to eventually notice when the other half of the
network connection has disappeared without sending a reset. I'm not
sure how to do that through APR, but it's probably not too difficult.
As a matter of process, you should never assume that a piece of code is
present for no reason when you have the means to investigate why it's
there. Using svn blame, you could have (after tracing back through the
code movement in r22238) seen that the code was introduced in r7532, and
correctly interpreted it as part of the pipelining support.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2355831
Received on 2009-05-27 18:52:25 CEST