Enable valgrind open FD check and fix open FDs after shutdown

plugins/ua_network_tcp.c

@@ -199,6 +199,7 @@ connection_recv(UA_Connection *connection, UA_ByteString *response,
     /* The remote side closed the connection */
     if(ret == 0) {
         UA_ByteString_deleteMembers(response);
+        connection->close(connection);
         return UA_STATUSCODE_BADCONNECTIONCLOSED;
     }
 
@@ -734,6 +735,8 @@ UA_ClientConnectionTCP(UA_ConnectionConfig conf,
      * want to try to connect. So use a loop and retry until timeout is
      * reached. */
     do {
+
+        connection.state = UA_CONNECTION_OPENING;
         /* Get a socket */
         clientsockfd = socket(server->ai_family,
                               server->ai_socktype,

src/server/ua_server_discovery.c

@@ -30,6 +30,7 @@ register_server_with_discovery_server(UA_Server *server,
         UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_CLIENT,
                      "Connecting to the discovery server failed with statuscode %s",
                      UA_StatusCode_name(retval));
+        UA_Client_disconnect(client);
         UA_Client_delete(client);
         return retval;
     }
@@ -66,8 +67,11 @@ register_server_with_discovery_server(UA_Server *server,
     size_t nl_discurls = server->config.networkLayersSize;
     size_t total_discurls = config_discurls + nl_discurls;
     request.server.discoveryUrls = (UA_String*)UA_alloca(sizeof(UA_String) * total_discurls);
-    if (!request.server.discoveryUrls)
+    if (!request.server.discoveryUrls) {
+        UA_Client_disconnect(client);
+        UA_Client_delete(client);
         return UA_STATUSCODE_BADOUTOFMEMORY;
+    }
     request.server.discoveryUrlsSize = total_discurls;
 
     for(size_t i = 0; i < config_discurls; ++i)

src/server/ua_services_discovery_multicast.c

@@ -343,6 +343,8 @@ initMulticastDiscoveryServer(UA_Server* server) {
 void destroyMulticastDiscoveryServer(UA_Server* server) {
     mdnsd_shutdown(server->mdnsDaemon);
     mdnsd_free(server->mdnsDaemon);
+    if (server->mdnsSocket > 0)
+        CLOSESOCKET(server->mdnsSocket);
 }
 
 static void

tests/CMakeLists.txt

@@ -50,10 +50,12 @@ endif()
 # Unit Test Definition Macro
 # For now we need to disable the libc freeres. See https://github.com/open62541/open62541/pull/1003#issuecomment-315045143
 # This also requires to disable the phtread cache with no-nptl-pthread-stackcache
-set(VALGRIND_FLAGS --quiet --trace-children=yes --leak-check=full --run-libc-freeres=no --sim-hints=no-nptl-pthread-stackcache)
+set(VALGRIND_FLAGS --quiet --trace-children=yes --leak-check=full --run-libc-freeres=no --sim-hints=no-nptl-pthread-stackcache --track-fds=yes)
 macro(add_test_valgrind TEST_NAME)
     if(UA_ENABLE_VALGRIND_UNIT_TESTS)
-        add_test(${TEST_NAME} valgrind --error-exitcode=1 --suppressions=${PROJECT_SOURCE_DIR}/tools/valgrind_suppressions.supp ${VALGRIND_FLAGS} ${ARGN})
+        set(VALGRIND_LOG ${TESTS_BINARY_DIR}/${TEST_NAME}.log)
+        set(VALGRIND_CMD valgrind --error-exitcode=1 --suppressions=${PROJECT_SOURCE_DIR}/tools/valgrind_suppressions.supp ${VALGRIND_FLAGS} --log-file=${VALGRIND_LOG} ${ARGN})
+        add_test(${TEST_NAME} ${PYTHON_EXECUTABLE} ${PROJECT_SOURCE_DIR}/tools/valgrind_check_error.py ${VALGRIND_LOG} ${VALGRIND_CMD})
     else()
         add_test(${TEST_NAME} ${ARGN})
     endif()

tests/check_types_range.c

@@ -96,6 +96,7 @@ int main(void) {
     suite_add_tcase(s, tc);
 
     SRunner *sr = srunner_create(s);
+    srunner_set_fork_status(sr, CK_NOFORK);
     srunner_run_all (sr, CK_NORMAL);
     int number_failed = srunner_ntests_failed(sr);
     srunner_free(sr);

tests/server/check_server_userspace.c

@@ -161,7 +161,7 @@ START_TEST(Server_set_customHostname) {
         ck_assert_int_eq(nl->discoveryUrl.length, len);
         ck_assert(strncmp(discoveryUrl, (char*)nl->discoveryUrl.data, len)==0);
     }
-
+    UA_Server_run_shutdown(server);
     UA_Server_delete(server);
     UA_ServerConfig_delete(config);
 }

tools/valgrind_check_error.py

@@ -0,0 +1,91 @@
+#!/usr/bin/env python
+
+# coding: UTF-8
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+# This script checks the valgrind output for errors.
+# The track-fds does not cause an error if there are too many FDs open,
+# therefore we parse the output and fail if there are open FDs
+
+import sys
+import subprocess
+import os.path
+import re
+
+logfile = sys.argv[1]
+
+valgrind_command = ' '.join('"' + item + '"' for item in sys.argv[2:])
+
+# Execute a command and output its stdout text
+def execute(command):
+    process = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+
+    # Poll process for new output until finished
+    while True:
+        nextline = process.stdout.readline().decode('utf-8')
+        if nextline == '' and process.poll() is not None:
+            break
+        sys.stdout.write(nextline)
+        sys.stdout.flush()
+
+    return process.returncode
+
+ret_code = execute(valgrind_command)
+
+if not os.path.isfile(logfile):
+    print("### PYTHON ERROR: Valgrind logfile does not exist: " + logfile)
+    exit(1)
+
+log_content = ""
+with open(logfile, 'r') as content_file:
+    log_content = content_file.read()
+
+if len(log_content) == 0:
+    print("### PYTHON ERROR: Valgrind logfile is empty: " + logfile)
+    exit(1)
+
+# Try to parse the output. Look for the following line:
+# ==17054== FILE DESCRIPTORS: 5 open at exit.
+descriptors_re = re.compile(r"^==(\d+)==\s+FILE DESCRIPTORS: (\d+) open at exit.$", re.MULTILINE)
+m = descriptors_re.match(log_content)
+
+if not m:
+    print("### PYTHON ERROR: File descriptors header not found: " + logfile)
+    print(log_content)
+    exit(1)
+
+log_content = descriptors_re.sub('', log_content)
+
+valgrind_number = m.group(1)
+open_count = int(m.group(2))
+
+# Remove the open file descriptors which are inherited from parent. they look like this:
+#==21343== Open file descriptor 3: /home/user/open62541/build/bin/tests/discovery.log
+#==21343==    <inherited from parent>
+#==21343==
+#==21343== Open file descriptor 2:
+#==21343==    <inherited from parent>
+#==21343==
+#==21343== Open file descriptor 1:
+#==21343==    <inherited from parent>
+#==21343==
+#==21343== Open file descriptor 0: /dev/pts/1
+#==21343==    <inherited from parent>
+
+replace_re = re.compile(r"^=="+str(valgrind_number)+r"==\s+Open file descriptor \d+:\s*[^\s]*$\n^=="+str(valgrind_number)+r"==\s+<inherited from parent>$\n(^=="+str(valgrind_number)+r"==\s+$\n)*", re.MULTILINE)
+log_content = replace_re.sub('', log_content)
+
+# Valgrind detected a memleak if ret_code != 0
+if ret_code != 0:
+    print(log_content)
+    exit(ret_code)
+
+# No issues by valgrind
+if log_content.isspace():
+    exit(0)
+
+# There is something fishy in the valgrind output, so error-exit
+print(log_content)
+exit(1)