From: Ian Jackson Date: Wed, 28 Jan 2015 23:17:03 +0000 (+0000) Subject: Portability: Fix assumption about read() and write() on connecting sockets X-Git-Tag: adns-1.5.1~12 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~mdw/git/adns/commitdiff_plain/777763cb0994893c42f4030a442d3d44a403b8fa?ds=sidebyside;hp=777763cb0994893c42f4030a442d3d44a403b8fa Portability: Fix assumption about read() and write() on connecting sockets Without adns_if_noautosys, adns would attempt read() on its TCP client socket immediately after getting EINPROGRESS from connect(), and assume that EAGAIN/EWOULDBLOCK means the socket is connected. This is actually not correct on any platform that I'm aware of. However, on Linux, write() on a socket which is being connected returns EAGAIN, so everything seemed to work - adns would think that the socket's window was full and wait for it to become writeable. On many other platforms, write() on such a socket returns ENOTCONN. The result is that adns's TCP support may fail to work properly on such platforms, especially if the nameserver is not localhost. (A connect to a suitable server on localhost often completes immediately, which avoids exposing the bug.) adns might fail to be able to do TCP at all. We detect the completion (successful or otherwise) of connect() by selecting the fd (as contemplated by SuS). This might expose us to spurious fd writeability indications, if such things exist, but the nonblocking connect API demands that they don't at least in this case. We always do this select check, in adns_processwriteable. In theory this is sometimes unnecessary, because adns_processwriteable's caller has probably just got a writeability indication from poll or select. But adns should not assume that its caller will never feed it spurious events, and there seems little point optimising away one syscall per tcp connection (given that adns reuses the connection where possible.) This behaviour naturally causes a lot of the regression tests to fail. So in this commit we also update all the regression tests. This has been done in a programmatic way, by running perl -i ./update-extra-select case-*.sys update-extra-select is supplied in this commit. It is a (hopefully easy to understand) script which adds a select just before every applicable read. The pretended select always reports that the fd is useable, which is what would justify adns's subsequent behaviour. (Most of the tests run with adns_if_noautosys - since that's the default and has to be toggled off - and in those cases the socket has in any case just been reported as writeable.) Note that regress/update-extra-select is not idempotent. Here, it has been used exactly once. (It is going to be deleted again in the next commit.) Signed-off-by: Ian Jackson ---