Skip to content

Commit

Permalink
Fix no router segfault (#549)
Browse files Browse the repository at this point in the history
* feat: add rc func that decrement without clearing

* build: add no_router test

* fix: missing newline eof
  • Loading branch information
jean-roland committed Jul 19, 2024
1 parent 64f2a09 commit a30a33b
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 2 deletions.
14 changes: 14 additions & 0 deletions .github/workflows/build-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,17 @@ jobs:
- 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
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Build & test pico
run: |
sudo apt install -y ninja-build
CMAKE_GENERATOR=Ninja make
python3 ./build/tests/no_router.py
timeout-minutes: 5
3 changes: 2 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ set(Z_FEATURE_SUBSCRIPTION 1 CACHE STRING "Toggle subscription feature")
set(Z_FEATURE_QUERY 1 CACHE STRING "Toggle query feature")
set(Z_FEATURE_QUERYABLE 1 CACHE STRING "Toggle queryable feature")
set(Z_FEATURE_RAWETH_TRANSPORT 0 CACHE STRING "Toggle raw ethernet transport feature")
set(Z_FEATURE_INTEREST 0 CACHE STRING "Toggle interest feature") # Toggle to 1 when protocol changes are merged
set(Z_FEATURE_INTEREST 1 CACHE STRING "Toggle interest feature")
add_definition(Z_FEATURE_MULTI_THREAD=${Z_FEATURE_MULTI_THREAD})
add_definition(Z_FEATURE_PUBLICATION=${Z_FEATURE_PUBLICATION})
add_definition(Z_FEATURE_SUBSCRIPTION=${Z_FEATURE_SUBSCRIPTION})
Expand Down Expand Up @@ -405,6 +405,7 @@ if(UNIX OR MSVC)
configure_file(${PROJECT_SOURCE_DIR}/tests/fragment.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/fragment.py COPYONLY)
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)

enable_testing()
add_test(z_data_struct_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_data_struct_test)
Expand Down
16 changes: 16 additions & 0 deletions include/zenoh-pico/collections/refcount.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,22 @@ extern "C++" {
z_free(p->in); \
return true; \
} \
static inline _Bool name##_rc_decr(name##_rc_t *p) { \
if (_ZP_RC_OP_DECR_AND_CMP_STRONG(1)) { \
if (_ZP_RC_OP_DECR_AND_CMP_WEAK(1)) { \
return false; \
} \
_ZP_RC_OP_SYNC \
z_free(p->in); \
return true; \
} \
if (_ZP_RC_OP_DECR_AND_CMP_WEAK(1)) { \
return false; \
} \
_ZP_RC_OP_SYNC \
z_free(p->in); \
return true; \
} \
static inline name##_weak_t name##_weak_null(void) { \
name##_weak_t p; \
p.in = NULL; \
Expand Down
2 changes: 1 addition & 1 deletion src/api/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ int8_t z_open(z_owned_session_t *zs, z_owned_config_t *config) {
// Open session
int8_t ret = _z_open(&zs->_rc, &config->_val);
if (ret != _Z_RES_OK) {
_z_session_rc_drop(&zs->_rc);
_z_session_rc_decr(&zs->_rc);
z_session_null(zs);
z_config_drop(config);
return ret;
Expand Down
155 changes: 155 additions & 0 deletions tests/no_router.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import argparse
import os
from signal import SIGINT
import subprocess
import sys
import time

# Specify the directory for the binaries
DIR_EXAMPLES = "build/examples"


def test_all():
print("*** Pub & sub test ***")
test_status = 0

# Expected output & status
z_expected_status = 255
z_expected_output = '''Opening session...
Unable to open session!'''

print("Start subscriber")
# Start z_sub
z_sub_command = f"stdbuf -oL -eL ./{DIR_EXAMPLES}/z_sub"
z_sub_process = subprocess.Popen(
z_sub_command, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
)
# Wait for z_sub to finish
z_sub_process.wait()

print("Start publisher")
# Start z_pub
z_pub_command = f"stdbuf -oL -eL ./{DIR_EXAMPLES}/z_pub"
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("Start queryable")
# Start z_queryable
z_queryable_command = f"stdbuf -oL -eL ./{DIR_EXAMPLES}/z_queryable"
z_queryable_process = subprocess.Popen(
z_queryable_command,
shell=True,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)
# Wait for z_queryable to finish
z_queryable_process.wait()

print("Start query")
# Start z_query
z_query_command = f"stdbuf -oL -eL ./{DIR_EXAMPLES}/z_get"
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("Check publisher status & output")
# Check the exit status of z_pub
z_pub_status = z_pub_process.returncode
if z_pub_status == z_expected_status:
print("z_pub status valid")
else:
print(f"z_pub status invalid, expected: {z_expected_status}, received: {z_pub_status}")
test_status = 1

# Check output of z_pub
z_pub_output = z_pub_process.stdout.read()
if z_expected_output in z_pub_output:
print("z_pub output valid")
else:
print("z_pub output invalid:")
print(f"Expected: \"{z_expected_output}\"")
print(f"Received: \"{z_pub_output}\"")
test_status = 1

print("Check subscriber status & output")
# Check the exit status of z_sub
z_sub_status = z_sub_process.returncode
if z_sub_status == z_expected_status:
print("z_sub status valid")
else:
print(f"z_sub status invalid, expected: {z_expected_status}, received: {z_sub_status}")
test_status = 1

# Check output of z_sub
z_sub_output = z_sub_process.stdout.read()
if z_expected_output in z_sub_output:
print("z_sub output valid")
else:
print("z_sub output invalid:")
print(f"Expected: \"{z_expected_output}\"")
print(f"Received: \"{z_sub_output}\"")
test_status = 1

print("Check query status & output")
# Check the exit status of z_query
z_query_status = z_query_process.returncode
if z_query_status == z_expected_status:
print("z_query status valid")
else:
print(f"z_query status invalid, expected: {z_expected_status}," f" received: {z_query_status}")
test_status = 1

# Check output of z_query
z_query_output = z_query_process.stdout.read()
if z_expected_output in z_query_output:
print("z_query output valid")
else:
print("z_query output invalid:")
print(f'Expected: "{z_expected_output}"')
print(f'Received: "{z_query_output}"')
test_status = 1

print("Check queryable status & output")
# Check the exit status of z_queryable
z_queryable_status = z_queryable_process.returncode
if z_queryable_status == z_expected_status:
print("z_queryable status valid")
else:
print(f"z_queryable status invalid, expected: {z_expected_status}," f" received: {z_queryable_status}")
test_status = 1

# Check output of z_queryable
z_queryable_output = z_queryable_process.stdout.read()
if z_expected_output in z_queryable_output:
print("z_queryable output valid")
else:
print("z_queryable output invalid:")
print(f'Expected: "{z_expected_output}"')
print(f'Received: "{z_queryable_output}"')
test_status = 1

# Return value
return test_status


if __name__ == "__main__":
EXIT_STATUS = 0

# Test all examples
if test_all() == 1:
EXIT_STATUS = 1

# Exit
sys.exit(EXIT_STATUS)
8 changes: 8 additions & 0 deletions tests/z_refcount_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,13 @@ void test_overflow(void) {
assert(dwk1.in == NULL);
}

void test_decr(void) {
_dummy_rc_t drc1 = _dummy_rc_new();
_dummy_rc_t drc2 = _dummy_rc_clone(&drc1);
assert(!_dummy_rc_decr(&drc2));
assert(_dummy_rc_decr(&drc1));
}

int main(void) {
test_rc_null();
test_rc_size();
Expand All @@ -239,5 +246,6 @@ int main(void) {
test_weak_copy();
test_weak_upgrade();
test_overflow();
test_decr();
return 0;
}

0 comments on commit a30a33b

Please sign in to comment.