From 405eb4256c54bc7fb76b1d8fd194f0a98778a7f7 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 13 Sep 2024 14:41:57 +0200 Subject: [PATCH 1/6] fix: memory leak on session open fail --- src/api/api.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/api/api.c b/src/api/api.c index 068780e20..b39198f54 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -930,6 +930,7 @@ int8_t z_open(z_owned_session_t *zs, z_moved_config_t *config, const z_open_opti _z_session_rc_decr(&zs->_rc); z_internal_session_null(zs); z_config_drop(config); + z_free(s); return ret; } // Clean up From 8ffd80ac08802cb6fc04995f4ac3121982566d75 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 13 Sep 2024 15:27:38 +0200 Subject: [PATCH 2/6] fix: encoding_convert memory leak --- src/api/api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/api.c b/src/api/api.c index b39198f54..83916d78f 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -324,7 +324,7 @@ static int8_t _z_encoding_convert_into_string(const z_loaned_encoding_t *encodin (void)strncat(value, _z_string_data(&encoding->schema), _z_string_len(&encoding->schema)); } // Fill container - s->_val = _z_string_alias_str(value); + s->_val = _z_string_from_str_custom_deleter(value, _z_delete_context_default()); return _Z_RES_OK; } From b289dd0a11cf07a3bc034111b82ef23950ec24ad Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 13 Sep 2024 15:38:16 +0200 Subject: [PATCH 3/6] tests: add memory leak test --- CMakeLists.txt | 1 + tests/memory_leak.py | 177 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 tests/memory_leak.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 8d13bc1bc..5e224af3d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -440,6 +440,7 @@ if(UNIX OR MSVC) configure_file(${PROJECT_SOURCE_DIR}/tests/single_thread.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/single_thread.py COPYONLY) configure_file(${PROJECT_SOURCE_DIR}/tests/attachment.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/attachment.py COPYONLY) configure_file(${PROJECT_SOURCE_DIR}/tests/no_router.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/no_router.py COPYONLY) + configure_file(${PROJECT_SOURCE_DIR}/tests/memory_leak.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/memory_leak.py COPYONLY) enable_testing() add_test(z_data_struct_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_data_struct_test) diff --git a/tests/memory_leak.py b/tests/memory_leak.py new file mode 100644 index 000000000..02501315e --- /dev/null +++ b/tests/memory_leak.py @@ -0,0 +1,177 @@ +import argparse +import os +from signal import SIGINT +import subprocess +import sys +import time +import re + +# Specify the directory for the binaries +DIR_EXAMPLES = "build/examples" + +NO_LEAK_OUTPUT = "All heap blocks were freed -- no leaks are possible" + + +def failure_mode(): + test_status = 0 + # Start binary + print("Start binary") + z_pub_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/z_pub -m peer" + z_pub_process = subprocess.Popen( + z_pub_command, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True + ) + # Wait for process to finish + z_pub_process.wait() + # Check output + print("Check output") + z_pub_output = z_pub_process.stderr.read() + if NO_LEAK_OUTPUT in z_pub_output: + print("Failure mode output valid") + else: + print("Failure mode output invalid:") + print(f"Received: \"{z_pub_output}\"") + test_status = 1 + # Return value + return test_status + + +def pub_and_sub(pub_cmd, sub_cmd): + test_status = 0 + + print("Start subscriber") + # Start z_sub in the background + z_sub_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/" + sub_cmd + + z_sub_process = subprocess.Popen( + z_sub_command, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True + ) + # Introduce a delay to ensure z_sub starts + time.sleep(2) + + print("Start publisher") + # Start z_pub + z_pub_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/" + pub_cmd + z_pub_process = subprocess.Popen( + z_pub_command, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True + ) + # Wait for z_pub to finish + z_pub_process.wait() + + print("Stop subscriber") + time.sleep(2) + if z_sub_process.poll() is None: + # send SIGINT to group + z_sub_process_gid = os.getpgid(z_sub_process.pid) + os.killpg(z_sub_process_gid, SIGINT) + + # Wait for z_sub to finish + z_sub_process.wait() + + print("Check outputs") + # Check output of z_pub + z_pub_output = z_pub_process.stderr.read() + if NO_LEAK_OUTPUT in z_pub_output: + print("z_pub output valid") + else: + print("z_pub output invalid:") + print(f"Received: \"{z_pub_output}\"") + test_status = 1 + + # Check output of z_sub + z_sub_output = z_sub_process.stderr.read() + if NO_LEAK_OUTPUT in z_sub_output: + print("z_sub output valid") + else: + print("z_sub output invalid:") + print(f"Received: \"{z_sub_output}\"") + test_status = 1 + # Return value + return test_status + + +def query_and_queryable(query_cmd, queryable_cmd): + test_status = 0 + print("Start queryable") + # Start z_queryable in the background + z_queryable_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/" + queryable_cmd + z_queryable_process = subprocess.Popen( + z_queryable_command, + shell=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + + # Introduce a delay to ensure z_queryable starts + time.sleep(2) + + print("Start query") + # Start z_query + z_query_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/" + query_cmd + z_query_process = subprocess.Popen( + z_query_command, + shell=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + # Wait for z_query to finish + z_query_process.wait() + + print("Stop queryable") + time.sleep(2) + if z_queryable_process.poll() is None: + # send SIGINT to group + z_quaryable_process_gid = os.getpgid(z_queryable_process.pid) + os.killpg(z_quaryable_process_gid, SIGINT) + + # Wait for z_queryable to finish + z_queryable_process.wait() + + print("Check outputs") + # Check output of z_query + z_query_output = z_query_process.stderr.read() + if NO_LEAK_OUTPUT in z_query_output: + print("z_query output valid") + else: + print("z_query output invalid:") + print(f'Received: "{z_query_output}"') + test_status = 1 + + # Check output of z_queryable + z_queryable_output = z_queryable_process.stderr.read() + if NO_LEAK_OUTPUT in z_queryable_output: + print("z_queryable output valid") + else: + print("z_queryable output invalid:") + print(f'Received: "{z_queryable_output}"') + test_status = 1 + # Return status + return test_status + + +if __name__ == "__main__": + EXIT_STATUS = 0 + + # Test failure mode + print("*** Failure mode ***") + if failure_mode() == 1: + EXIT_STATUS = 1 + # Test pub and sub examples + print("*** Pub & sub test ***") + if pub_and_sub(f'z_pub -n 1', f'z_sub -n 1') == 1: + EXIT_STATUS = 1 + print("*** Pub & sub attachment test ***") + if pub_and_sub(f'z_pub_attachment -n 1', f'z_sub_attachment -n 1') == 1: + EXIT_STATUS = 1 + # Test query and queryable examples + print("*** Query & queryable test ***") + if query_and_queryable(f'z_get', f'z_queryable -n 1') == 1: + EXIT_STATUS = 1 + print("*** Query & queryable attachment test ***") + if query_and_queryable(f'z_get_attachment -v Something', f'z_queryable_attachment -n 1') == 1: + EXIT_STATUS = 1 + # Exit + sys.exit(EXIT_STATUS) From c700d6163c4146635958fb57e44e28aa7102b247 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 13 Sep 2024 15:44:14 +0200 Subject: [PATCH 4/6] ci: add memory leak test workflow --- .github/workflows/build-check.yaml | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.github/workflows/build-check.yaml b/.github/workflows/build-check.yaml index 59a3f172c..19537f18d 100644 --- a/.github/workflows/build-check.yaml +++ b/.github/workflows/build-check.yaml @@ -242,6 +242,39 @@ jobs: if: always() run: kill ${{ steps.run-zenoh.outputs.zenohd-pid }} + memory_leak_test: + needs: zenoh_build + name: Test examples memory leak + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Download Zenoh artifacts + uses: actions/download-artifact@v4 + with: + name: ${{ needs.zenoh_build.outputs.artifact-name }} + + - name: Unzip Zenoh artifacts + run: unzip ${{ needs.zenoh_build.outputs.artifact-name }} -d zenoh-standalone + + - id: run-zenoh + name: Run Zenoh router + run: | + RUST_LOG=debug ./zenoh-standalone/zenohd & + echo "zenohd-pid=$!" >> $GITHUB_OUTPUT + + - name: Build project and run test + run: | + sudo apt install -y ninja-build valgrind + CMAKE_GENERATOR=Ninja make + python3 ./build/tests/memory_leak.py + timeout-minutes: 5 + + - name: Kill Zenoh router + if: always() + run: kill ${{ steps.run-zenoh.outputs.zenohd-pid }} + no_routeur: name: Test examples without router runs-on: ubuntu-latest From e1c18c4a36c34956a7d8a999c4b89ee829eb96d0 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 13 Sep 2024 15:53:37 +0200 Subject: [PATCH 5/6] tests: modularize failure mode --- tests/memory_leak.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/memory_leak.py b/tests/memory_leak.py index 02501315e..34c49c9e5 100644 --- a/tests/memory_leak.py +++ b/tests/memory_leak.py @@ -12,11 +12,11 @@ NO_LEAK_OUTPUT = "All heap blocks were freed -- no leaks are possible" -def failure_mode(): +def failure_mode(fail_cmd): test_status = 0 # Start binary print("Start binary") - z_pub_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/z_pub -m peer" + z_pub_command = f"stdbuf -oL -eL valgrind ./{DIR_EXAMPLES}/" + fail_cmd z_pub_process = subprocess.Popen( z_pub_command, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True ) @@ -157,7 +157,7 @@ def query_and_queryable(query_cmd, queryable_cmd): # Test failure mode print("*** Failure mode ***") - if failure_mode() == 1: + if failure_mode(f'z_pub -m peer') == 1: EXIT_STATUS = 1 # Test pub and sub examples print("*** Pub & sub test ***") From 24e456bd6245807a3734831793dfeec3fb447e41 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 13 Sep 2024 16:41:43 +0200 Subject: [PATCH 6/6] ci: use dedicated valgrind action --- .github/workflows/build-check.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-check.yaml b/.github/workflows/build-check.yaml index 19537f18d..70a87854d 100644 --- a/.github/workflows/build-check.yaml +++ b/.github/workflows/build-check.yaml @@ -264,9 +264,12 @@ jobs: RUST_LOG=debug ./zenoh-standalone/zenohd & echo "zenohd-pid=$!" >> $GITHUB_OUTPUT + - name: Install valgrind + uses: taiki-e/install-action@valgrind + - name: Build project and run test run: | - sudo apt install -y ninja-build valgrind + sudo apt install -y ninja-build CMAKE_GENERATOR=Ninja make python3 ./build/tests/memory_leak.py timeout-minutes: 5