From 5c509831b8b556e75f6947bd8841af547ecd2bc5 Mon Sep 17 00:00:00 2001 From: TapTap Date: Thu, 17 Sep 2026 23:44:41 +0200 Subject: [PATCH] fix(test): robust valgrind detection + per-suite io-fd reset; init per_dir_filter_count The post-merge valgrind job on dev hangs. Root cause: the CI valgrind step exports FASTSYNC_UNDER_VALGRIND=1, but nothing read it, and the /proc/self/maps "vgpreload" probe is unreliable on valgrind 3.22 (the guest's maps no longer list the tool's own libraries). So the fork-based unit tests ran under valgrind anyway; tests that call io_set_fds() left the thread-local read/write descriptors pointing at a closed test pipe, and a later send_n_data()/receive_n_data() call was silently redirected to those stale fds (legacy_session() prefers the globals, which the stdin/stdout SSH server requires). Later tests only worked by fd-reuse luck; under valgrind the fd numbers no longer coincide, so the read blocked forever on an empty pipe. - test_utils.h: honor FASTSYNC_UNDER_VALGRIND (already set by ci.yaml) and keep the maps scan as a best-effort fallback. Reset io_set_fds(-1, -1) at the start of every RUN_TEST so one suite cannot leak descriptor redirection into the next. - test_iconv.c: skip the forking wire-string roundtrip under valgrind like the other fork-based tests. - config.c: initialize per_dir_filter_count in config_set_defaults. The field was never initialized, so -F/-FF counting read uninitialized heap (valgrind: conditional jump on uninitialised value at client_cli.c:1464) and could count from garbage. Verified with the CI-equivalent command (FASTSYNC_UNDER_VALGRIND=1 valgrind --leak-check=full --show-leak-kinds=definite --error-exitcode=1): completes with 0 errors (previously hung >80 min). Unit 43/43; full integration 729 passed; cppcheck and clang-format clean. --- src/shared/config.c | 1 + tests/test_iconv.c | 5 ++++- tests/test_utils.h | 19 ++++++++++++++++--- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/shared/config.c b/src/shared/config.c index 6adc800..03dcebd 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -76,6 +76,7 @@ static void config_set_defaults(Config* config) { config->from0 = false; config->cvs_exclude = false; config->per_dir_filter = false; + config->per_dir_filter_count = 0; config->one_file_system = false; config->no_implied_dirs = false; config->dirs = false; diff --git a/tests/test_iconv.c b/tests/test_iconv.c index faee11b..da31243 100644 --- a/tests/test_iconv.c +++ b/tests/test_iconv.c @@ -213,5 +213,8 @@ void test_iconv() { test_iconv_wire_sender_converts_local_to_remote(); test_iconv_wire_receiver_converts_remote_to_local(); test_iconv_wire_disabled_passthrough(); - test_iconv_wire_str_roundtrip(); + // This subtest forks to exercise the wire string handshake; the instrumented + // parent is too slow under valgrind for the child's blocking reads. + if (!is_running_under_valgrind()) + test_iconv_wire_str_roundtrip(); } \ No newline at end of file diff --git a/tests/test_utils.h b/tests/test_utils.h index 4d5f3d3..93d87ed 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -6,10 +6,22 @@ #include #include -// Detect if running under valgrind by checking /proc/self/maps for vgpreload. -// This is used to skip fork-based tests that are incompatible with valgrind -// (the instrumented parent runs too slowly, causing pipe timeouts). +// Reset the thread-local protocol descriptor redirection installed by +// io_set_fds(), so a suite that leaks a test pipe's fds cannot redirect a later +// suite's raw send_n_data()/receive_n_data() to the wrong descriptor. +void io_set_fds(int read_fd, int write_fd); + +// Detect if running under valgrind. The CI valgrind step exports +// FASTSYNC_UNDER_VALGRIND=1; a /proc/self/maps scan is the fallback for a local +// valgrind run (newer valgrind versions can hide their own mappings from the +// guest, so the "vgpreload" match is not reliable on every version -- set +// FASTSYNC_UNDER_VALGRIND=1 when invoking valgrind by hand). Used to skip +// fork-based tests incompatible with valgrind, whose instrumented parent runs +// too slowly and causes pipe timeouts. static inline bool is_running_under_valgrind(void) { + const char* env = getenv("FASTSYNC_UNDER_VALGRIND"); + if (env && env[0] != '\0' && strcmp(env, "0") != 0) + return true; FILE* f = fopen("/proc/self/maps", "r"); if (!f) return false; @@ -31,6 +43,7 @@ extern bool current_test_failed; printf("Running %s...\n", #test_func); \ tests_run++; \ current_test_failed = false; \ + io_set_fds(-1, -1); \ test_func(); \ if (current_test_failed) { \ tests_failed++; \ -- 2.54.0