soreceive_stream: issues with O_NONBLOCK
Mikolaj Golub
trociny at freebsd.org
Sun Jul 3 15:00:23 UTC 2011
Hi,
Trying soreceive_stream I found that many applications (like firefox, pidgin,
gnus) might hang in soreceive_stream/sbwait.
It was shown up that the issue was with O_NONBLOCK connections -- they blocked
in recv() when should not have been.
This can be checked with this simple test:
http://people.freebsd.org/~trociny/test_nonblock.c
In soreceive_stream we have the following code that looks wrong:
1968 /* Socket buffer is empty and we shall not block. */
1969 if (sb->sb_cc == 0 &&
1970 ((sb->sb_flags & SS_NBIO) || (flags & (MSG_DONTWAIT|MSG_NBIO)))) {
1971 error = EAGAIN;
1972 goto out;
1973 }
It should check so->so_state agains SS_NBIO, not sb->sb_flags. But just
changing this is not enough. This check is called too early -- before checking
that socket state is not SBS_CANTRCVMORE. As a result, if the peer closes the
connection recv() returns EAGAIN instead of 0. See this example:
http://people.freebsd.org/~trociny/test_close.c
So I moved the "nonblock" check below SBS_CANTRCVMORE check and ended up with
this patch:
http://people.freebsd.org/~trociny/uipc_socket.c.soreceive_stream.patch
It works for me fine.
Also, this part looks wrong:
1958 /* We will never ever get anything unless we are connected. */
1959 if (!(so->so_state & (SS_ISCONNECTED|SS_ISDISCONNECTED))) {
1960 /* When disconnecting there may be still some data left. */
1961 if (sb->sb_cc > 0)
1962 goto deliver;
1963 if (!(so->so_state & SS_ISDISCONNECTED))
1964 error = ENOTCONN;
1965 goto out;
1966 }
Why we check in 1959 that state is not SS_ISDISCONNECTED? If it is valid then
the check at 1963 is useless becase it will be always true. Shouldn't it be
something like below?
if (!(so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING))) {
/* When disconnecting there may be still some data left. */
if (sb->sb_cc > 0)
goto deliver;
error = ENOTCONN;
goto out;
}
(I don't see why we souldn't set ENOTCONN if the state is SS_ISDISCONNECTED).
And the last :-). Currently, to try soreceive_stream one need to rebuild kernel
with TCP_SORECEIVE_STREAM and then set tunable net.inet.tcp.soreceive_stream.
Why do we need TCP_SORECEIVE_STREAM option? Wouldn't tunable be enough? It
would simplify trying soreceive_stream by users and we might have more
testing/feedback.
--
Mikolaj Golub
More information about the freebsd-net
mailing list