From bb9d9a9e813242275e868457c4b30f40f13d65e8 Mon Sep 17 00:00:00 2001
From: Simon Cornish <simon@liveoak.local>
Date: Wed, 2 Dec 2009 12:31:46 -0800
Subject: [PATCH] Implement a non-blocking gen_sctp:connect

This patch extends the functionality of gen_sctp:connect by prividing a non-blocking connect call when Timeout = 0.

Especially when using multi-homed connections, the current implementation
has a race-condition that can cause the wrong
sctp_association_change record to be returned to the
application.

With this patch, when an application calls connect with
Timeout = 0 then the application is responsible for receiving
the #sctp_assoc_change{} message and correctly determining the
connect it originated from (for example, by examining the
remote host and/or port). The application should have at least
{active, once} or be prepared to gen_sctp:recv the connect
result.

The behaviour is unchanged if Timeout /= 0.

Example application code:
try_connect(A) ->
    cancel_timer(A),
    case gen_sctp:connect(A#assoc.sock, A#assoc.remote_addr_order, A#assoc.remote_port, [{active, once}], 0) of
	ok ->
	    Timer = erlang:send_after(?ASSOC_RETRY_TIME,self(), ?TimerMsg(A, assoc_retry_expiry)),
	    %% these results indicate connection in progress
	    {connecting, A#assoc{timer = Timer}};
	Err ->
	    {Err, A}
    end.
---
 lib/kernel/doc/src/gen_sctp.xml |  7 ++++++-
 lib/kernel/src/inet_sctp.erl    | 20 ++++++++++++++------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/lib/kernel/doc/src/gen_sctp.xml b/lib/kernel/doc/src/gen_sctp.xml
index de41178a17e7..8667e26ac981 100644
--- a/lib/kernel/doc/src/gen_sctp.xml
+++ b/lib/kernel/doc/src/gen_sctp.xml
@@ -155,7 +155,7 @@
       </desc>
     </func>
     <func>
-      <name>connect(Socket, Addr, Port, [Opt], Timeout) -&gt; {ok, Assoc} | {error, posix()}</name>
+      <name>connect(Socket, Addr, Port, [Opt], Timeout) -&gt; ok | {ok, Assoc} | {error, posix()}</name>
       <fsummary>Establish a new association for the socket <c>Socket</c>, with a peer (SCTP server socket)</fsummary>
       <type>
         <v>Socket  = sctp_socket()</v>
@@ -185,6 +185,11 @@
         inbound_streams   = int(),
         assoc_id          = assoc_id()
   }        </pre>
+
+	If <c>Timeout</c> is 0, then the result of <c>connect/*</c> is
+	ok. The caller must receive or recv/* the sctp_assoc_change
+	message to determine the result of the connect call.
+
         <p>The number of outbound and inbound streams can be set by
           giving an <c>sctp_initmsg</c> option to <c>connect</c>
           as in:</p>
diff --git a/lib/kernel/src/inet_sctp.erl b/lib/kernel/src/inet_sctp.erl
index 30c0e85dd903..4a216dd3b351 100644
--- a/lib/kernel/src/inet_sctp.erl
+++ b/lib/kernel/src/inet_sctp.erl
@@ -64,6 +64,14 @@ close(S) ->
 listen(S, Flag) ->
     prim_inet:listen(S, Flag).
 
+%% A non-blocking connect is implemented when the initial call to
+%% gen_sctp:connect has Timeout = 0. Then the application is
+%% responsible for receiving the #sctp_assoc_change{} message and
+%% correctly determining the connect it originated from (for
+%% example, by examining the remote host and/or port).
+%% The application should have at least {active, once} or be prepared
+%% to gen_sctp:recv the connect result.
+
 connect(S, Addr, Port, Opts, Timer) ->
     case prim_inet:chgopts(S, Opts) of
 	ok ->
@@ -71,9 +79,9 @@ connect(S, Addr, Port, Opts, Timer) ->
 		{ok,Active} ->
 		    Timeout = inet:timeout(Timer),
 		    case prim_inet:connect(S, Addr, Port, Timeout) of
-			ok ->
+			ok when Timeout /= 0 ->
 			    connect_get_assoc(S, Addr, Port, Active, Timer);
-			Err1 -> Err1
+			OkOrErr1 -> OkOrErr1
 		    end;
 		Err2 -> Err2
 	    end;
@@ -89,10 +97,10 @@ connect(S, Addr, Port, Opts, Timer) ->
 %% connect_get_assoc/5 below mistakes it for an invalid response
 %% for a socket in {active,false} or {active,once} modes.
 %%
-%% In {active,true} mode it probably gets right, but it is
-%% a blocking connect that is implemented even for {active,true},
-%% and that may be a shortcoming. A non-blocking connect
-%% would be nice to have.
+%% In {active,true} mode the window of time for the race is smaller,
+%% but it is possible and also it is a blocking connect that is
+%% implemented even for {active,true}, and that may be a
+%% shortcoming.
 
 connect_get_assoc(S, Addr, Port, false, Timer) ->
     case recv(S, inet:timeout(Timer)) of