Re: Dpi_connect_socket retry

Hi,

  Finally I had the time to review this issue in more detail...

On Sun, Dec 09, 2012 at 02:58:43PM +0400, [email protected] 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.-