Hi, Finally I had the time to review this issue in more detail... On Sun, Dec 09, 2012 at 02:58:43PM +0400, p37sitdu at lavabit.com wrote:
Dpi_connect_socket function from src/IO/dpi.c (also duplicated in test/cookies.c) has retry argument. It is set to TRUE on first call, and Dpi_connect_socket calls itself recursively with retry == FALSE in case of error.
This retry function is not working. If connection fails, at line 645 sock_fd is set, but ret is still set to -1. Even if the second attempt to connect is successful, -1 is returned and conected sock_fd is lost.
It is possible to fix it by adding "ret = sock_fd", but I think it is better to just remove retries completely. It is unlikely that connecting using loopback interface will fail on first try and succeed on retry.
Yes, this code is obsolete now. It was meant for Unix domain sockets (when dpis/dpid used them). Sometimes the filename lingered in the filesystem after a dpi or server crash. With unlink&retry it was possible to recover then. Details in patchset: 7faa2c7a544f (Nov, 2009 ;) Removed in the repo (see below).
Also, comment says "the server closes sock_fd on error". However, sock_fd should still be closed by client to free allocated file descriptor. OS don't know if Dillo is going to read from it (in which case error such as ECONNRESET should happen) so file descriptor can't be reused.
File descriptor should be closed in case of error in a_Dpip_build_cmd too.
The file descriptor is properly closed by its CCC afterwards (Visible by defining VERBOSE in chain.c). The CCC manages the error propagation/handling, in both the requesting and answer branches.
Patch to fix this function and its calls in src/IO/dpi.c and test/cookies.c is attached.
No need to fix, it works properly. -- Cheers Jorge.-