fix(test): robust valgrind detection + per-suite io-fd reset; init per_dir_filter_count
CI / lint (pull_request) Successful in 1m41s
CI / sanitizers (address) (pull_request) Skipped
CI / sanitizers (undefined) (pull_request) Skipped
CI / fuzz-build (pull_request) Skipped
CI / coverage (pull_request) Skipped
CI / valgrind (pull_request) Skipped
CI / build-and-test (pull_request) Successful in 46s

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.
This commit is contained in:
2026-09-17 23:44:41 +02:00
parent ee57aeea4a
commit 5c509831b8
3 changed files with 21 additions and 4 deletions
+1
View File
@@ -76,6 +76,7 @@ static void config_set_defaults(Config* config) {
config->from0 = false; config->from0 = false;
config->cvs_exclude = false; config->cvs_exclude = false;
config->per_dir_filter = false; config->per_dir_filter = false;
config->per_dir_filter_count = 0;
config->one_file_system = false; config->one_file_system = false;
config->no_implied_dirs = false; config->no_implied_dirs = false;
config->dirs = false; config->dirs = false;
+3
View File
@@ -213,5 +213,8 @@ void test_iconv() {
test_iconv_wire_sender_converts_local_to_remote(); test_iconv_wire_sender_converts_local_to_remote();
test_iconv_wire_receiver_converts_remote_to_local(); test_iconv_wire_receiver_converts_remote_to_local();
test_iconv_wire_disabled_passthrough(); test_iconv_wire_disabled_passthrough();
// 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(); test_iconv_wire_str_roundtrip();
} }
+16 -3
View File
@@ -6,10 +6,22 @@
#include <string.h> #include <string.h>
#include <stdbool.h> #include <stdbool.h>
// Detect if running under valgrind by checking /proc/self/maps for vgpreload. // Reset the thread-local protocol descriptor redirection installed by
// This is used to skip fork-based tests that are incompatible with valgrind // io_set_fds(), so a suite that leaks a test pipe's fds cannot redirect a later
// (the instrumented parent runs too slowly, causing pipe timeouts). // 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) { 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"); FILE* f = fopen("/proc/self/maps", "r");
if (!f) if (!f)
return false; return false;
@@ -31,6 +43,7 @@ extern bool current_test_failed;
printf("Running %s...\n", #test_func); \ printf("Running %s...\n", #test_func); \
tests_run++; \ tests_run++; \
current_test_failed = false; \ current_test_failed = false; \
io_set_fds(-1, -1); \
test_func(); \ test_func(); \
if (current_test_failed) { \ if (current_test_failed) { \
tests_failed++; \ tests_failed++; \