[socket-nif] Correct state checks when calling read and write functions

The read and write functions now *only* use the isReadable and
isWritable fields, respectively, to decide if the "socket" can be read
from and written to (previously the state field was used outside of
the mutex lock).

Also made it possible to enable / disable test suite groups individually,
via environment variables (ESOCK_TEST_...). Specifically, the ttest
group has been excluded by default (takes to long a time).

OTP-15549

2 files changed, 74 insertions, 64 deletions

diff --git a/erts/emulator/nifs/common/socket_nif.c b/erts/emulator/nifs/common/socket_nif.c
index 44aa44f5fc..9375e9c005 100644
--- a/erts/emulator/nifs/common/socket_nif.c
+++ b/erts/emulator/nifs/common/socket_nif.c
@@ -4250,6 +4250,12 @@ ERL_NIF_TERM nopen(ErlNifEnv* env,
     descP->type     = type;
     descP->protocol = protocol;
 
+    /* Does this apply to other types? Such as RAW? */
+    if (type == SOCK_DGRAM) {
+        descP->isReadable = TRUE;
+        descP->isWritable = TRUE;
+    }
+
     /*
      * Should we keep track of sockets (resources) in some way?
      * Doing it here will require mutex to ensure data integrity,
@@ -4544,20 +4550,27 @@ ERL_NIF_TERM nconnect(ErlNifEnv*        env,
 {
     int code, save_errno = 0;
 
-    /* Verify that we are where in the proper state */
+    /* 
+     * <KOLLA>
+     *
+     * We should look both the read and write mutex:es...
+     *
+     * </KOLLA>
+     *
+     * Verify that we are where in the proper state */
 
     if (!IS_OPEN(descP)) {
-        SSDBG( descP, ("SOCKET", "nif_sendto -> not open\r\n") );
+        SSDBG( descP, ("SOCKET", "nif_connect -> not open\r\n") );
         return esock_make_error(env, atom_exbadstate);
     }
 
     if (IS_CONNECTED(descP)) {
-        SSDBG( descP, ("SOCKET", "nif_sendto -> already connected\r\n") );
+        SSDBG( descP, ("SOCKET", "nif_connect -> already connected\r\n") );
         return esock_make_error(env, atom_eisconn);
     }
 
     if (IS_CONNECTING(descP)) {
-        SSDBG( descP, ("SOCKET", "nif_sendto -> already connecting\r\n") );
+        SSDBG( descP, ("SOCKET", "nif_connect -> already connecting\r\n") );
         return esock_make_error(env, esock_atom_einval);
     }
     
@@ -4566,7 +4579,7 @@ ERL_NIF_TERM nconnect(ErlNifEnv*        env,
                         descP->addrLen);
     save_errno = sock_errno();
 
-    SSDBG( descP, ("SOCKET", "nif_sendto -> connect result: %d, %d\r\n",
+    SSDBG( descP, ("SOCKET", "nif_connect -> connect result: %d, %d\r\n",
                    code, save_errno) );
 
     if (IS_SOCKET_ERROR(code) &&
@@ -4580,7 +4593,9 @@ ERL_NIF_TERM nconnect(ErlNifEnv*        env,
                descP, NULL, ref);
         return esock_make_ok2(env, ref);
     } else if (code == 0) {                 /* ok we are connected */
-        descP->state = SOCKET_STATE_CONNECTED;
+        descP->state      = SOCKET_STATE_CONNECTED;
+        descP->isReadable = TRUE;
+        descP->isWritable = TRUE;
         /* Do we need to do somthing for "active" mode?
          * Is there even such a thing *here*?
          */
@@ -4637,7 +4652,9 @@ ERL_NIF_TERM nfinalize_connection(ErlNifEnv*        env,
         return esock_make_error_errno(env, error);
     }
 
-    descP->state = SOCKET_STATE_CONNECTED;
+    descP->state      = SOCKET_STATE_CONNECTED;
+    descP->isReadable = TRUE;
+    descP->isWritable = TRUE;
 
     return esock_atom_ok;
 }
@@ -4968,7 +4985,9 @@ ERL_NIF_TERM naccept_listening(ErlNifEnv*        env,
                descP, NULL, esock_atom_undefined);
 #endif
 
-        accDescP->state = SOCKET_STATE_CONNECTED;
+        accDescP->state      = SOCKET_STATE_CONNECTED;
+        accDescP->isReadable = TRUE;
+        accDescP->isWritable = TRUE;
 
         return esock_make_ok2(env, accRef);
     }
@@ -5108,7 +5127,9 @@ ERL_NIF_TERM naccept_accepting(ErlNifEnv*        env,
                descP, NULL, esock_atom_undefined);
 #endif
 
-        accDescP->state = SOCKET_STATE_CONNECTED;
+        accDescP->state      = SOCKET_STATE_CONNECTED;
+        accDescP->isReadable = TRUE;
+        accDescP->isWritable = TRUE;
 
         /* Check if there are waiting acceptors (popping the acceptor queue) */
 
@@ -5182,9 +5203,6 @@ ERL_NIF_TERM nif_send(ErlNifEnv*         env,
         return enif_make_badarg(env);
     }
 
-    if (IS_CLOSED(descP) || IS_CLOSING(descP))
-        return esock_make_error(env, atom_closed);
-    
     SSDBG( descP,
            ("SOCKET", "nif_send -> args when sock = %d:"
             "\r\n   Socket:       %T"
@@ -5193,9 +5211,6 @@ ERL_NIF_TERM nif_send(ErlNifEnv*         env,
             "\r\n   eFlags:       %d"
             "\r\n", descP->sock, sockRef, sendRef, sndData.size, eflags) );
 
-    if (!IS_CONNECTED(descP))
-        return esock_make_error(env, atom_enotconn);
-
     if (!esendflags2sendflags(eflags, &flags))
         return enif_make_badarg(env);
 
@@ -5315,9 +5330,6 @@ ERL_NIF_TERM nif_sendto(ErlNifEnv*         env,
         return enif_make_badarg(env);
     }
     
-    if (IS_CLOSED(descP) || IS_CLOSING(descP))
-        return esock_make_error(env, atom_closed);
-    
     SSDBG( descP,
            ("SOCKET", "nif_sendto -> args when sock = %d:"
             "\r\n   Socket:       %T"
@@ -5328,12 +5340,6 @@ ERL_NIF_TERM nif_sendto(ErlNifEnv*         env,
             "\r\n",
             descP->sock, sockRef, sendRef, sndData.size, eSockAddr, eflags) );
 
-    /* THIS TEST IS NOT CORRECT!!! */
-    if (!IS_OPEN(descP)) {
-        SSDBG( descP, ("SOCKET", "nif_sendto -> not open (%u)\r\n", descP->state) );
-        return esock_make_error(env, esock_atom_einval);
-    }
-
     if (!esendflags2sendflags(eflags, &flags)) {
         SSDBG( descP, ("SOCKET", "nif_sendto -> sendflags decode failed\r\n") );
         return esock_make_error(env, esock_atom_einval);
@@ -5447,9 +5453,6 @@ ERL_NIF_TERM nif_sendmsg(ErlNifEnv*         env,
         return enif_make_badarg(env);
     }
     
-    if (IS_CLOSED(descP) || IS_CLOSING(descP))
-        return esock_make_error(env, atom_closed);
-    
     SSDBG( descP,
            ("SOCKET", "nif_sendmsg -> args when sock = %d:"
             "\r\n   Socket:  %T"
@@ -5458,10 +5461,6 @@ ERL_NIF_TERM nif_sendmsg(ErlNifEnv*         env,
             "\r\n",
             descP->sock, argv[0], sendRef, eflags) );
 
-    /* THIS TEST IS NOT CORRECT!!! */
-    if (!IS_OPEN(descP))
-        return esock_make_error(env, esock_atom_einval);
-
     if (!esendflags2sendflags(eflags, &flags))
         return esock_make_error(env, esock_atom_einval);
 
@@ -5745,12 +5744,6 @@ ERL_NIF_TERM nif_recv(ErlNifEnv*         env,
         return enif_make_badarg(env);
     }
     
-    if (IS_CLOSED(descP) || IS_CLOSING(descP))
-        return esock_make_error(env, atom_closed);
-    
-    if (!IS_CONNECTED(descP))
-        return esock_make_error(env, atom_enotconn);
-
     if (!erecvflags2recvflags(eflags, &flags))
         return enif_make_badarg(env);
 
@@ -5893,9 +5886,6 @@ ERL_NIF_TERM nif_recvfrom(ErlNifEnv*         env,
         return enif_make_badarg(env);
     }
 
-    if (IS_CLOSED(descP) || IS_CLOSING(descP))
-        return esock_make_error(env, atom_closed);
-    
     SSDBG( descP,
            ("SOCKET", "nif_recvfrom -> args when sock = %d:"
             "\r\n   Socket:  %T"
@@ -6062,9 +6052,6 @@ ERL_NIF_TERM nif_recvmsg(ErlNifEnv*         env,
         return enif_make_badarg(env);
     }
     
-    if (IS_CLOSED(descP) || IS_CLOSING(descP))
-        return esock_make_error(env, atom_closed);
-    
     SSDBG( descP,
            ("SOCKET", "nif_recvmsg -> args when sock = %d:"
             "\r\n   Socket:  %T"
@@ -6292,6 +6279,8 @@ ERL_NIF_TERM nclose(ErlNifEnv*        env,
 
         descP->closeLocal = TRUE;
         descP->state      = SOCKET_STATE_CLOSING;
+        descP->isReadable = FALSE;
+        descP->isWritable = FALSE;
         doClose           = TRUE;
     }
 
@@ -15842,7 +15831,7 @@ SocketDescriptor* alloc_descriptor(SOCKET sock, HANDLE event)
         descP->currentWriterP = NULL; // currentWriter not used
         descP->writersQ.first = NULL;
         descP->writersQ.last  = NULL;
-        descP->isWritable     = TRUE;
+        descP->isWritable     = FALSE; // TRUE;
         descP->writePkgCnt    = 0;
         descP->writeByteCnt   = 0;
         descP->writeTries     = 0;
@@ -15854,7 +15843,7 @@ SocketDescriptor* alloc_descriptor(SOCKET sock, HANDLE event)
         descP->currentReaderP = NULL; // currentReader not used
         descP->readersQ.first = NULL;
         descP->readersQ.last  = NULL;
-        descP->isReadable     = TRUE;
+        descP->isReadable     = FALSE; // TRUE;
         descP->readPkgCnt     = 0;
         descP->readByteCnt    = 0;
         descP->readTries      = 0;
diff --git a/erts/emulator/test/socket_SUITE.erl b/erts/emulator/test/socket_SUITE.erl
index 2fbc0dddad..01d332c34e 100644
--- a/erts/emulator/test/socket_SUITE.erl
+++ b/erts/emulator/test/socket_SUITE.erl
@@ -454,14 +454,29 @@ suite() ->
      {timetrap,{minutes,1}}].
 
 all() -> 
-    [
-     {group, api},
-     {group, socket_closure},
-     {group, traffic},
-     {group, ttest}
-
-     %% {group, tickets}
-    ].
+    Groups = [{api,            "ESOCK_TEST_API",        include},
+	      {socket_closure, "ESOCK_TEST_SOCK_CLOSE", include},
+	      {traffic,        "ESOCK_TEST_TRAFFIC",    include},
+	      {ttest,          "ESOCK_TEST_TTEST",      exclude}],
+    [use_group(Group, Env, Default) || {Group, Env, Default} <- Groups].
+
+use_group(Group, Env, Default) ->
+	case os:getenv(Env) of
+	    false when (Default =:= include) ->
+		[{group, Group}];
+	    false ->
+		[];
+	    Val ->
+		case list_to_atom(string:to_lower(Val)) of
+		    Use when (Use =:= include) orelse 
+			     (Use =:= enable) orelse 
+			     (Use =:= true) ->
+			[{group, Group}];
+		    _ ->
+			[]
+		end
+	end.
+    
 
 groups() -> 
     [{api,                 [], api_cases()},
@@ -10919,12 +10934,12 @@ tpp_udp_server_handler_msg_exchange_loop(Sock, Send, Recv,
     %%         socket:setopt(Sock, otp, debug, true); 
     %%     true -> ok
     %% end,
-    case tpp_udp_recv_req(Sock, Recv) of
+    try tpp_udp_recv_req(Sock, Recv) of
         {ok, Msg, RecvSz, From} ->
             NewStart = if (Start =:= undefined) -> ?LIB:timestamp(); 
                           true -> Start end,
             %% ?SEV_IPRINT("[~w] received - now try send", [N]),
-            case tpp_udp_send_rep(Sock, Send, Msg, From) of
+            try tpp_udp_send_rep(Sock, Send, Msg, From) of
                 {ok, SendSz} ->
                     tpp_udp_server_handler_msg_exchange_loop(Sock, Send, Recv,
                                                              N+1,
@@ -10934,15 +10949,10 @@ tpp_udp_server_handler_msg_exchange_loop(Sock, Send, Recv,
                 {error, SReason} ->
                     ?SEV_EPRINT("send (~w): ~p", [N, SReason]),
                     exit({send, SReason, N})
+	    catch
+		SC:SE:SS ->
+		    exit({send, {SC, SE, SS}, N})
             end;
-        %% {error, timeout} ->
-        %%     ?SEV_IPRINT("timeout(~w) - try again", [N]),
-        %%     case Send(Sock, list_to_binary("ping")) of
-        %%         ok ->
-        %%             exit({'ping-send', ok, N});
-        %%         {error, Reason} ->
-        %%             exit({'ping-send', Reason, N})
-        %%     end;
         {error, closed} ->
             ?SEV_IPRINT("closed - we are done: ~w, ~w, ~w",
                         [N, Sent, Received]),
@@ -10951,6 +10961,9 @@ tpp_udp_server_handler_msg_exchange_loop(Sock, Send, Recv,
         {error, RReason} ->
             ?SEV_EPRINT("recv (~w): ~p", [N, RReason]),
             exit({recv, RReason, N})
+    catch
+	RC:RE:RS ->
+	    exit({recv, {RC, RE, RS}, N})	
     end.
   
 
@@ -11033,9 +11046,11 @@ tpp_udp_recv_rep(Sock, Recv) ->
     tpp_udp_recv(Sock, Recv, ?TPP_REPLY).
 
 tpp_udp_recv(Sock, Recv, Tag) ->
-    case Recv(Sock, 0) of
+    %% ok = socket:setopt(Sock, otp, debug, true),
+    try Recv(Sock, 0) of
         {ok, {Source, <<Tag:32/integer, Sz:32/integer, Data/binary>> = Msg}} 
           when (Sz =:= size(Data)) ->
+	    %% ok = socket:setopt(Sock, otp, debug, false),
             %% We got it all
             %% ?SEV_IPRINT("tpp_udp_recv -> got all: "
             %%             "~n   Source:     ~p"
@@ -11045,11 +11060,17 @@ tpp_udp_recv(Sock, Recv, Tag) ->
             %%             [Source, Tag, Sz, size(Data)]),
             {ok, Data, size(Msg), Source};
         {ok, {_Source, <<Tag:32/integer, Sz:32/integer, Data/binary>>}} ->
+	    %% ok = socket:setopt(Sock, otp, debug, false),
             {error, {invalid_msg, Sz, size(Data)}};
         {ok, {_, <<Tag:32/integer, _/binary>>}} ->
+	    %% ok = socket:setopt(Sock, otp, debug, false),
             {error, {invalid_msg_tag, Tag}};
         {error, _} = ERROR ->
+	    %% ok = socket:setopt(Sock, otp, debug, false),
             ERROR
+    catch
+	C:E:S ->
+	    {error, {catched, C, E, S}}
     end.
 
 tpp_udp_send_req(Sock, Send, Data, Dest) ->