That previous one-line change was apparently not quite enough. I think I
still see it occasionally hang, just not all the time. I'm guessing that
this flag is sometimes already clear on entry to this function, so if we
want to force a non-blocking read we must explicitly set it:
--- srclib/apr/network_io/unix/sockopt.c.orig 2012-11-07
08:10:09.000000000 -0800
+++ srclib/apr/network_io/unix/sockopt.c 2016-02-04 15:04:24.833986781
-0800
@@ -100,11 +100,20 @@
}
}
/* must disable the incomplete read support if we disable
- * a timeout
+ * a timeout...
*/
- if (t <= 0) {
+ if (t < 0) {
sock->options &= ~APR_INCOMPLETE_READ;
}
+ else if (t == 0)
+ {
+ /* and must set the APR_INCOMPLETE_READ flag if non-blocking
+ * read was requested because apr_socket_recv() does a
+ * blocking read if this flag is clear ...
+ */
+ sock->options |= APR_INCOMPLETE_READ;
+ }
+
sock->timeout = t;
return APR_SUCCESS;
}
Does the APR_INCOMPLETE_READ flag have another purpose that is
orthogonal to controlling blocking behaviour? If yes, then I'm a little
concerned to overload its use like this. Should there be a different
dedicated flag to control blocking versus non-blocking behaviour? Or is
the patch safe as is?
Thanks,
Joachim
On 2016-02-04 14:53, Joachim Achtzehnter wrote:
On 2016-02-04 0:54, Yann Ylavic wrote:
On Thu, Feb 4, 2016 at 9:41 AM, Yann Ylavic <ylavic....@gmail.com> wrote:
Doesn't the socket_bucket_read() call (frame #3) enter
apr_socket_timeout_set(p, 0), and hence sononblock() which puts the
socket in O_NONBLOCK?
and resets APR_INCOMPLETE_READ.
Yes, it does:
if (t <= 0) {
sock->options &= ~APR_INCOMPLETE_READ;
}
But isn't this precisely the problem? The socket_buffer_read()
implementation in "srclib/apr/network_io/unix/sendrecv.c" uses select()
only if the APR_INCOMPLETE_READ flag is SET:
if (sock->options & APR_INCOMPLETE_READ) {
sock->options &= ~APR_INCOMPLETE_READ;
goto do_select;
}
If this flag is clear it goes into the blocking read loop instead:
do {
rv = read(sock->socketdes, buf, (*len));
} while (rv == -1 && errno == EINTR);
How about this small patch:
--- srclib/apr/network_io/unix/sockopt.c.orig 2012-11-07
08:10:09.000000000 -0800
+++ srclib/apr/network_io/unix/sockopt.c 2016-02-04
14:20:35.755530111 -0800
@@ -102,7 +102,7 @@
/* must disable the incomplete read support if we disable
* a timeout
*/
- if (t <= 0) {
+ if (t < 0) {
sock->options &= ~APR_INCOMPLETE_READ;
}
sock->timeout = t;
This seems to get it working, but I'm not sure if there are other
reasons, not related to blocking versus non-blocking behaviour, for the
flag to get cleared when timeout is zero.
Thanks,
Joachim
--
joac...@kraut.ca http://www.kraut.ca
