add UA_Client_close and the ability to reactivate a session (#1444)

* add UA_Client_close and the ability to reactivate a session

* add some check

* remove double socket close introduce in #1417

* remove double close

examples/client_connect_loop.c

@@ -82,7 +82,7 @@ int main(void) {
             UA_Variant_hasScalarType(&value, &UA_TYPES[UA_TYPES_DATETIME])) {
             UA_DateTime raw_date = *(UA_DateTime *) value.data;
             UA_DateTimeStruct dts = UA_DateTime_toStruct(raw_date);
-            UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "date is: %u-%u-%u %u:%u:%u.%03u\n",
+            UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "date is: %u-%u-%u %u:%u:%u.%03u",
                         dts.day, dts.month, dts.year, dts.hour, dts.min, dts.sec, dts.milliSec);
         }
         UA_Variant_deleteMembers(&value);

include/ua_client.h

@@ -105,10 +105,14 @@ UA_StatusCode UA_EXPORT
 UA_Client_connect_username(UA_Client *client, const char *endpointUrl,
                            const char *username, const char *password);
 
-/* Close a connection to the selected server */
+/* Disconnect and close a connection to the selected server */
 UA_StatusCode UA_EXPORT
 UA_Client_disconnect(UA_Client *client);
 
+/* Close a connection to the selected server */
+UA_StatusCode UA_EXPORT
+UA_Client_close(UA_Client *client);
+
 /* Renew the underlying secure channel */
 UA_StatusCode UA_EXPORT
 UA_Client_manuallyRenewSecureChannel(UA_Client *client);

plugins/ua_network_tcp.c

@@ -144,6 +144,8 @@ connection_releaserecvbuffer(UA_Connection *connection,
 
 static UA_StatusCode
 connection_write(UA_Connection *connection, UA_ByteString *buf) {
+    if (connection->state == UA_CONNECTION_CLOSED)
+        return UA_STATUSCODE_BADCONNECTIONCLOSED;
     /* Prevent OS signals when sending to a closed socket */
     int flags = 0;
 #ifdef MSG_NOSIGNAL
@@ -176,6 +178,8 @@ connection_write(UA_Connection *connection, UA_ByteString *buf) {
 static UA_StatusCode
 connection_recv(UA_Connection *connection, UA_ByteString *response,
                 UA_UInt32 timeout) {
+    if (connection->state == UA_CONNECTION_CLOSED)
+        return UA_STATUSCODE_BADCONNECTIONCLOSED;
     /* Listen on the socket for the given timeout until a message arrives */
     if(timeout > 0) {
         fd_set fdset;
@@ -292,6 +296,8 @@ ServerNetworkLayerTCP_freeConnection(UA_Connection *connection) {
  * socket is returned from select. */
 static void
 ServerNetworkLayerTCP_close(UA_Connection *connection) {
+    if (connection->state == UA_CONNECTION_CLOSED)
+        return;
     shutdown((SOCKET)connection->sockfd, 2);
     connection->state = UA_CONNECTION_CLOSED;
 }
@@ -649,7 +655,6 @@ ServerNetworkLayerTCP_deleteMembers(UA_ServerNetworkLayer *nl) {
     ConnectionEntry *e, *e_tmp;
     LIST_FOREACH_SAFE(e, &layer->connections, pointers, e_tmp) {
         LIST_REMOVE(e, pointers);
-        ServerNetworkLayerTCP_close(&e->connection);
         CLOSESOCKET(e->connection.sockfd);
         UA_free(e);
     }
@@ -684,6 +689,8 @@ UA_ServerNetworkLayerTCP(UA_ConnectionConfig conf, UA_UInt16 port) {
 
 static void
 ClientNetworkLayerTCP_close(UA_Connection *connection) {
+    if (connection->state == UA_CONNECTION_CLOSED)
+        return;
     shutdown((SOCKET)connection->sockfd, 2);
     CLOSESOCKET(connection->sockfd);
     connection->state = UA_CONNECTION_CLOSED;
@@ -864,7 +871,8 @@ UA_ClientConnectionTCP(UA_ConnectionConfig conf,
 
     if(!connected) {
         /* connection timeout */
-        ClientNetworkLayerTCP_close(&connection);
+        if (connection.state != UA_CONNECTION_CLOSED)
+            ClientNetworkLayerTCP_close(&connection);
         UA_LOG_WARNING(UA_Log_Stdout, UA_LOGCATEGORY_NETWORK,
                        "Trying to connect to %s timed out",
                        endpointUrl);

src/client/ua_client.c

@@ -291,8 +291,7 @@ receiveServiceResponse(UA_Client *client, void *response, const UA_DataType *res
         if(retval != UA_STATUSCODE_GOOD && retval != UA_STATUSCODE_GOODNONCRITICALTIMEOUT) {
             if(retval == UA_STATUSCODE_BADCONNECTIONCLOSED)
                 client->state = UA_CLIENTSTATE_DISCONNECTED;
-            else
-                UA_Client_disconnect(client);
+            UA_Client_close(client);
             break;
         }
     } while(!rd.received);
@@ -314,7 +313,7 @@ __UA_Client_Service(UA_Client *client, const void *request,
             respHeader->serviceResult = UA_STATUSCODE_BADREQUESTTOOLARGE;
         else
             respHeader->serviceResult = retval;
-        UA_Client_disconnect(client);
+        UA_Client_close(client);
         return;
     }
 
@@ -324,7 +323,7 @@ __UA_Client_Service(UA_Client *client, const void *request,
     retval = receiveServiceResponse(client, response, responseType, maxDate, &requestId);
     if (retval == UA_STATUSCODE_GOODNONCRITICALTIMEOUT){
         /* In synchronous service, if we have don't have a reply we need to close the connection */
-        UA_Client_disconnect(client);
+        UA_Client_close(client);
         retval = UA_STATUSCODE_BADCONNECTIONCLOSED;
     }
     if(retval != UA_STATUSCODE_GOOD)

src/client/ua_client_connect.c

@@ -97,9 +97,13 @@ HelAckHandshake(UA_Client *client) {
     /* Loop until we have a complete chunk */
     retval = UA_Connection_receiveChunksBlocking(conn, client, processACKResponse,
                                                  client->config.timeout);
-    if(retval != UA_STATUSCODE_GOOD)
+    if(retval != UA_STATUSCODE_GOOD){
         UA_LOG_INFO(client->config.logger, UA_LOGCATEGORY_NETWORK,
                     "Receiving ACK message failed");
+        if(retval == UA_STATUSCODE_BADCONNECTIONCLOSED)
+            client->state = UA_CLIENTSTATE_DISCONNECTED;
+        UA_Client_close(client);
+    }
     return retval;
 }
 
@@ -166,7 +170,7 @@ openSecureChannel(UA_Client *client, UA_Boolean renew) {
     if(retval != UA_STATUSCODE_GOOD) {
         UA_LOG_ERROR(client->config.logger, UA_LOGCATEGORY_SECURECHANNEL,
                      "Sending OPN message failed with error %s", UA_StatusCode_name(retval));
-        UA_Client_disconnect(client);
+        UA_Client_close(client);
         return retval;
     }
 
@@ -182,7 +186,7 @@ openSecureChannel(UA_Client *client, UA_Boolean renew) {
                                     &requestId);
                                     
     if(retval != UA_STATUSCODE_GOOD) {
-        UA_Client_disconnect(client);
+        UA_Client_close(client);
         return retval;
     }
 
@@ -362,7 +366,6 @@ UA_Client_connectInternal(UA_Client *client, const char *endpointUrl,
                           UA_Boolean endpointsHandshake, UA_Boolean createNewSession) {
     if(client->state >= UA_CLIENTSTATE_CONNECTED)
         return UA_STATUSCODE_GOOD;
-
     UA_ChannelSecurityToken_init(&client->channel.securityToken);
     client->channel.state = UA_SECURECHANNELSTATE_FRESH;
 
@@ -396,40 +399,44 @@ UA_Client_connectInternal(UA_Client *client, const char *endpointUrl,
         goto cleanup;
     client->state = UA_CLIENTSTATE_SECURECHANNEL;
 
-    /* There is no session to recover and we want to have a session */
-    if(createNewSession && UA_NodeId_equal(&client->authenticationToken, &UA_NODEID_NULL)) {
-        /* Get Endpoints */
-        if(endpointsHandshake) {
-            retval = getEndpoints(client);
+    /* Try to activate an existing Session for this SecureChannel */
+    if((!UA_NodeId_equal(&client->authenticationToken, &UA_NODEID_NULL)) && (createNewSession)) {
+        retval = activateSession(client);
+        if(retval == UA_STATUSCODE_BADSESSIONIDINVALID) {
+            /* Could not recover an old session. Remove authenticationToken */
+            UA_NodeId_deleteMembers(&client->authenticationToken);
+        }else{
             if(retval != UA_STATUSCODE_GOOD)
                 goto cleanup;
+            client->state = UA_CLIENTSTATE_SESSION;
+            return retval;
         }
-        /* Create a Session */
-        retval = createSession(client);
+    }else{
+        UA_NodeId_deleteMembers(&client->authenticationToken);
+    }
+
+    /* Get Endpoints */
+    if(endpointsHandshake) {
+        retval = getEndpoints(client);
         if(retval != UA_STATUSCODE_GOOD)
             goto cleanup;
     }
 
-    /* Activate the Session for this SecureChannel */
-    if(!UA_NodeId_equal(&client->authenticationToken, &UA_NODEID_NULL)) {
-        retval = activateSession(client);
-
-        /* Could not recover an old session. Create a new one */
-        if(retval == UA_STATUSCODE_BADSESSIONIDINVALID) {
-            retval = createSession(client);
-            if(retval != UA_STATUSCODE_GOOD)
-                goto cleanup;
-            retval = activateSession(client);
-        }
-
+    /* Create the Session for this SecureChannel */
+    if(createNewSession) {
+        retval = createSession(client);
         if(retval != UA_STATUSCODE_GOOD)
             goto cleanup;
-        client->state = UA_CLIENTSTATE_SESSION;
+        retval = activateSession(client);
     }
+
+    if(retval != UA_STATUSCODE_GOOD)
+        goto cleanup;
+    client->state = UA_CLIENTSTATE_SESSION;
     return retval;
 
 cleanup:
-    UA_Client_disconnect(client);
+    UA_Client_close(client);
     return retval;
 }
 
@@ -451,7 +458,8 @@ UA_StatusCode
 UA_Client_manuallyRenewSecureChannel(UA_Client *client) {
     UA_StatusCode retval = openSecureChannel(client, true);
     if(retval != UA_STATUSCODE_GOOD)
-        client->state = UA_CLIENTSTATE_DISCONNECTED;
+        UA_Client_close(client);
+
     return retval;
 }
 
@@ -486,6 +494,7 @@ sendCloseSecureChannel(UA_Client *client) {
     UA_SecureChannel_sendSymmetricMessage(channel, ++client->requestId,
                                           UA_MESSAGETYPE_CLO, &request,
                                           &UA_TYPES[UA_TYPES_CLOSESECURECHANNELREQUEST]);
+    UA_CloseSecureChannelRequest_deleteMembers(&request);
     UA_SecureChannel_deleteMembersCleanup(&client->channel);
 }
 
@@ -504,7 +513,22 @@ UA_Client_disconnect(UA_Client *client) {
         sendCloseSecureChannel(client);
 
     /* Close the TCP connection */
-    if(client->state >= UA_CLIENTSTATE_CONNECTED)
+    if(client->connection.state != UA_CONNECTION_CLOSED)
+        client->connection.close(&client->connection);
+
+    client->state = UA_CLIENTSTATE_DISCONNECTED;
+    return UA_STATUSCODE_GOOD;
+}
+
+UA_StatusCode
+UA_Client_close(UA_Client *client) {
+    client->requestHandle = 0;
+
+    if (client->state >= UA_CLIENTSTATE_SECURECHANNEL)
+        UA_SecureChannel_deleteMembersCleanup(&client->channel);
+
+    /* Close the TCP connection */
+    if(client->connection.state != UA_CONNECTION_CLOSED)
         client->connection.close(&client->connection);
 
     client->state = UA_CLIENTSTATE_DISCONNECTED;

src/ua_securechannel.c

@@ -96,7 +96,9 @@ UA_SecureChannel_deleteMembersCleanup(UA_SecureChannel *channel) {
 
     /* Detach from the connection and close the connection */
     if(channel->connection){
-        channel->connection->close(channel->connection);
+        if(channel->connection->state != UA_CONNECTION_CLOSED){
+            channel->connection->close(channel->connection);
+        }
         UA_Connection_detachSecureChannel(channel->connection);
     }
 

tests/client/check_client.c

@@ -7,11 +7,14 @@
 
 #include "ua_types.h"
 #include "ua_server.h"
+#include "ua_server_internal.h"
 #include "ua_client.h"
+#include "client/ua_client_internal.h"
 #include "ua_config_default.h"
 #include "ua_client_highlevel.h"
 #include "ua_network_tcp.h"
 #include "testing_clock.h"
+#include "testing_networklayers.h"
 #include "check.h"
 #include "thread_wrapper.h"
 
@@ -121,34 +124,115 @@ START_TEST(Client_renewSecureChannel) {
 } END_TEST
 
 START_TEST(Client_reconnect) {
-        UA_ClientConfig clientConfig = UA_ClientConfig_default;
-        clientConfig.timeout = 100;
-        UA_Client *client = UA_Client_new(clientConfig);
-        UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-
-        UA_Variant val;
-        UA_NodeId nodeId = UA_NODEID_STRING(1, "my.variable");
-        retval = UA_Client_readValueAttribute(client, nodeId, &val);
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-        UA_Variant_deleteMembers(&val);
-
-        // restart server to test reconnect
-        teardown();
-        setup();
-
-        retval = UA_Client_readValueAttribute(client, nodeId, &val);
-        ck_assert_uint_eq(retval, UA_STATUSCODE_BADCONNECTIONCLOSED);
-
-        retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
-        ck_assert_msg(retval == UA_STATUSCODE_GOOD, UA_StatusCode_name(retval));
-
-        retval = UA_Client_readValueAttribute(client, nodeId, &val);
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-        UA_Variant_deleteMembers(&val);
-
-        UA_Client_disconnect(client);
-        UA_Client_delete(client);
+    UA_ClientConfig clientConfig = UA_ClientConfig_default;
+    clientConfig.timeout = 100;
+    UA_Client *client = UA_Client_new(clientConfig);
+    UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+
+    UA_Variant val;
+    UA_NodeId nodeId = UA_NODEID_STRING(1, "my.variable");
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    UA_Variant_deleteMembers(&val);
+
+    // restart server to test reconnect
+    teardown();
+    setup();
+
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_BADCONNECTIONCLOSED);
+
+    retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_msg(retval == UA_STATUSCODE_GOOD, UA_StatusCode_name(retval));
+
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    UA_Variant_deleteMembers(&val);
+
+    UA_Client_disconnect(client);
+    UA_Client_delete(client);
+}
+END_TEST
+
+START_TEST(Client_activateSessionClose) {
+    // restart server
+    teardown();
+    setup();
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 0);
+
+    UA_ClientConfig clientConfig = UA_ClientConfig_default;
+    UA_Client *client = UA_Client_new(clientConfig);
+    UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 1);
+
+    UA_Variant val;
+    UA_NodeId nodeId = UA_NODEID_STRING(1, "my.variable");
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    UA_Variant_deleteMembers(&val);
+
+    UA_Client_close(client);
+
+    retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 1);
+
+    nodeId = UA_NODEID_STRING(1, "my.variable");
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    UA_Variant_deleteMembers(&val);
+
+    UA_Client_disconnect(client);
+    UA_Client_delete(client);
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 0);
+}
+END_TEST
+
+START_TEST(Client_activateSessionTimeout) {
+    // restart server
+    teardown();
+    setup();
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 0);
+
+    UA_Client *client = UA_Client_new(UA_ClientConfig_default);
+    UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 1);
+
+    UA_Variant val;
+    UA_Variant_init(&val);
+    UA_NodeId nodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS_STATE);
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    UA_Variant_deleteMembers(&val);
+
+    UA_Client_recv = client->connection.recv;
+    client->connection.recv = UA_Client_recvTesting;
+
+    /* Simulate network cable unplugged (no response from server) */
+    UA_Client_recvTesting_result = UA_STATUSCODE_GOODNONCRITICALTIMEOUT;
+
+    UA_Variant_init(&val);
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_BADCONNECTIONCLOSED);
+
+    ck_assert_msg(UA_Client_getState(client) == UA_CLIENTSTATE_DISCONNECTED);
+
+    UA_Client_recvTesting_result = UA_STATUSCODE_GOOD;
+    retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 1);
+
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+    UA_Variant_deleteMembers(&val);
+
+    UA_Client_delete(client);
+
+    ck_assert_uint_eq(server->sessionManager.currentSessionCount, 0);
 }
 END_TEST
 
@@ -163,6 +247,8 @@ static Suite* testSuite_Client(void) {
     tcase_add_checked_fixture(tc_client_reconnect, setup, teardown);
     tcase_add_test(tc_client_reconnect, Client_renewSecureChannel);
     tcase_add_test(tc_client_reconnect, Client_reconnect);
+    tcase_add_test(tc_client_reconnect, Client_activateSessionClose);
+    tcase_add_test(tc_client_reconnect, Client_activateSessionTimeout);
     suite_add_tcase(s,tc_client_reconnect);
     return s;
 }