GitLab

0ac7ed9e introduced a bug which causes network sockets not to be set into nonblocking mode when requested with shout_set_nonblocking(s, 1). This can be demonstrated simply by tracing an example nonblocking app.

Bad (current master HEAD):

$ strace -f -e fcntl,fcntl64,socket,connect ./nonblocking
(...)
socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 4
connect(4, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 4
connect(4, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 4
connect(4, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 4
connect(4, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = 0

Good (v2.4.3):

(...)
socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 4
fcntl(4, F_SETFL, O_RDONLY|O_NONBLOCK)  = 0
connect(4, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now i   n progress)

In the first case fcntl is never called and the socket stays in the default (blocking) mode.

This is due to the following condition in shout_connection_connect:

    if (con->nonblocking == SHOUT_BLOCKING_DEFAULT)
        shout_connection_set_nonblocking(con, shout_get_nonblocking(shout));

But as con->nonblocking is never initialized to this value, it's effectively if(0).

Adding a simple:

    con->nonblocking = SHOUT_BLOCKING_DEFAULT;

to shout_connection_new fixes the problem. But this turns the above condition into if(1), so is it really needed? It was not there before. What's the idea behind this?