[PATCH] Patch for bug 12105

[Jeremy Allison]

On Wed, Aug 03, 2016 at 03:27:12PM +0200, Ralph Böhme wrote:
> Hi!
> 
> Andreas pointed out that I introduced a regression to async_req that
> can cause smbclient (and other callers) to eat 100% CPU.
> 
> Attached patch fixes this for me:
> 
> $ bin/smbclient -L 10.10.10.10 -U%
> Connection to 10.10.10.10 failed (Error NT_STATUS_HOST_UNREACHABLE)
> 
> $ bin/smbclient -L www.heise.de -U%
> Connection to www.heise.de failed (Error NT_STATUS_IO_TIMEOUT)
> 
> Please review & push if ok.

Oh wow - this is a subtle one !

But yep - man page and OpenGroup page checks out...

      EINPROGRESS
              The  socket  is nonblocking and the connection cannot be completed immediately.

       EALREADY
              The socket is nonblocking and a previous connection attempt has not yet been completed.

http://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html

Who coud have guessed connect() had these semantics with
multiple calls (not me, obviously :-).

Reviewed-by: Jeremy Allison <jra at samba.org>

> From a45d111a22d4649a04a3e27b934737f220593eda Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 3 Aug 2016 15:00:45 +0200
> Subject: [PATCH] async_req: make async_connect_send() "reentrant"
> 
> Allow callers to pass in socket fds that where already passed to an
> earlier call of async_connect_send(). Callers expect this behaviour and
> it was working until 05d4dbda8357712cb81008e0d611fdb0e7239587 broke it.
> 
> The proper fix would be to change callers to close the fd and start from
> scratch with a fresh socket.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12105
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  lib/async_req/async_sock.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c
> index c14acf3..3af1748 100644
> --- a/lib/async_req/async_sock.c
> +++ b/lib/async_req/async_sock.c
> @@ -128,11 +128,21 @@ struct tevent_req *async_connect_send(
>  	}
>  
>  	/*
> -	 * The only errno indicating that the connect is still in
> -	 * flight is EINPROGRESS, everything else is an error
> +	 * The only errno indicating that an initial connect is still
> +	 * in flight is EINPROGRESS.
> +	 *
> +	 * We get EALREADY when someone calls us a second time for a
> +	 * given fd and the connect is still in flight (and returned
> +	 * EINPROGRESS the first time).
> +	 *
> +	 * This allows callers like open_socket_out_send() to reuse
> +	 * fds and call us with an fd for which the connect is still
> +	 * in flight. The proper thing to do for callers would be
> +	 * closing the fd and starting from scratch with a fresh
> +	 * socket.
>  	 */
>  
> -	if (errno != EINPROGRESS) {
> +	if (errno != EINPROGRESS && errno != EALREADY) {
>  		tevent_req_error(req, errno);
>  		return tevent_req_post(req, ev);
>  	}
> -- 
> 2.7.4
>