From d47b4286866a0ba759a9906a82a944ac11c59108 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Sat, 22 Aug 2026 22:49:16 +0700 Subject: [PATCH 1/2] run the threading rule rather than only the violation of it misuse.c checks what happens when a connection is used from two threads, which it can do from one thread because the progress watcher supplies the second. Nothing checked that the arrangement the header tells a host to use actually works, which needs real threads. Four cases. Eight threads with a connection each off one shared database, sixty four statements down each, all answering the same number. A connection moved from thread to thread with a join between every handoff, which is what a pool does. One connection in two threads at once, racing for real rather than through the watcher, where the only two allowed answers are ok and ZU_MISUSE_CONCURRENT and the connection has to be correct afterwards. And an interrupt from another thread, which the header calls the one exception to the sharing rule, landing while the connection is busy and leaving it fit to use. Its own file rather than more cases in misuse.c because the thread sanitizer and the address sanitizer cannot be linked into one binary. --- test/CMakeLists.txt | 15 ++ test/threads.c | 463 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 478 insertions(+) create mode 100644 test/threads.c diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ecf0edf..8ce361d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -18,6 +18,21 @@ if(UNIX) C_STANDARD_REQUIRED ON C_EXTENSIONS OFF) add_test(NAME misuse COMMAND misuse) + + # And the rule kept rather than broken, which needs real threads and + # is therefore the only file here that starts any. Separate from + # misuse.c because the thread sanitizer and the address sanitizer + # cannot be linked into one binary, so CI builds this tree twice and + # runs a different one of the two each time. + find_package(Threads REQUIRED) + add_executable(threads threads.c) + target_link_libraries(threads PRIVATE zu::zu zu_cpp_build_settings Threads::Threads) + target_include_directories(threads PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}") + set_target_properties(threads PROPERTIES + C_STANDARD 11 + C_STANDARD_REQUIRED ON + C_EXTENSIONS OFF) + add_test(NAME threads COMMAND threads) endif() # One executable per file, which is what gives ctest a name per file and diff --git a/test/threads.c b/test/threads.c new file mode 100644 index 0000000..32c82fd --- /dev/null +++ b/test/threads.c @@ -0,0 +1,463 @@ +/* The concurrency the ABI promises, run concurrently. + * + * The header opens with the rule the whole threading story rests on: a + * zu_database "holds no descriptor and no cache, so it is thread-safe + * and shareable", a zu_conn "is the state that cannot be shared", and + * "a host that queries from four threads opens one database and + * connects four times". + * + * Nothing here was running that. misuse.c checks what happens when the + * rule is broken, which it can do from one thread because the progress + * watcher supplies the second. This file checks that the rule works + * when it is kept, which needs real threads, and it is the only file in + * the suite that starts any. + * + * That is also what gives the thread sanitizer something to say, and + * why this is a file of its own rather than three more cases in + * misuse.c: CI runs this one under TSan and the rest under address and + * undefined behaviour, and those two instrumentations cannot be linked + * into one binary. + * + * What TSan can and cannot see here is worth being straight about. + * libzu is not instrumented, so a race entirely inside the engine's own + * memory is invisible to it. What it does see is every pthread + * primitive, because those are intercepted rather than compiled, and + * every access this file makes to its own memory from more than one + * thread. So it covers the half a host can get wrong, which is the half + * this repository is here to be right about. + */ +#include + +#include +#include +#include +#include +#include + +#include "harness.h" + +#define WORKERS 8 +#define ROUNDS 64 +#define PEOPLE 512 + +/* One array of people, read by every thread and written by none, which + * is the shape a host holding a dataframe arrives in. The header says + * of registered frames that "nothing stops the connections registering + * the same memory twice", and this is that sentence run eight ways at + * once. */ +static int64_t ids[PEOPLE]; + +/* What one worker was given and what it found. Every field is written + * by its own thread and read by the main one after a join, which is an + * ordering pthread_join establishes and TSan knows about. */ +struct worker { + pthread_t id; + zu_database *db; + int index; + int rounds; + int64_t answer; + zu_status failed_at; + const char *stage; +}; + +static void *count_people(void *arg) { + struct worker *w = (struct worker *)arg; + zu_conn *conn = NULL; + zu_frame *frame = NULL; + int i; + + /* A connection of its own, out of the shared database, which is the + * documented arrangement. */ + w->stage = "connect"; + w->failed_at = zu_connect(w->db, &conn, NULL); + if (w->failed_at != ZU_OK) { + return NULL; + } + /* And a frame of its own over the shared array, because a frame is a + * connection-shaped thing rather than a database-shaped one: "a frame + * is used from one thread, like the connection it registers on". The + * memory underneath is what is shared. */ + w->stage = "frame"; + w->failed_at = zu_frame_new_z("Person", PEOPLE, NULL, NULL, &frame, NULL); + if (w->failed_at == ZU_OK) { + w->stage = "column"; + w->failed_at = zu_frame_col_int(frame, "id", 2, ids, PEOPLE, 64, 1, 1, ZU_FRAME_PLAIN, NULL); + } + if (w->failed_at == ZU_OK) { + w->stage = "register"; + w->failed_at = zu_conn_register(conn, frame, NULL); + } + if (w->failed_at != ZU_OK) { + zu_frame_free(frame); + zu_conn_close(conn); + return NULL; + } + + w->stage = "query"; + for (i = 0; i < ROUNDS; i++) { + zu_result *res = NULL; + const zu_value *cell = NULL; + int64_t n = 0; + zu_status st = zu_query_z(conn, "MATCH (p:Person) RETURN count(*) AS n", &res, NULL); + if (st != ZU_OK) { + w->failed_at = st; + break; + } + if (zu_result_cell(res, 0, 0, &cell) != ZU_OK || zu_value_i64(cell, &n) != ZU_OK) { + w->stage = "cell"; + w->failed_at = ZU_MISUSE; + zu_result_free(res); + break; + } + zu_result_free(res); + w->answer = n; + w->rounds++; + } + zu_conn_close(conn); + zu_frame_free(frame); + return NULL; +} + +ZT_TEST(eight_threads_with_a_connection_each_read_one_database_and_agree) { + /* The supported shape, run hard enough to be worth running: one + * database handle shared by eight threads, a connection made inside + * each of them, and sixty four statements down each connection. + * + * All eight have to finish, and all eight have to answer 512, because + * nothing is writing. A wrong count from one thread and a right one + * from the other seven is the failure this is looking for, and it is + * the failure a locked-per-connection design is meant to make + * impossible. + * + * A frame rather than a file, so the threads contend on the engine's + * own structures rather than queue on a disk. */ + struct worker workers[WORKERS]; + zu_database *db = NULL; + int i; + + for (i = 0; i < PEOPLE; i++) { + ids[i] = i; + } + memset(workers, 0, sizeof workers); + + ZT_CHECK_EQ(zu_database_memory(NULL, &db, NULL), ZU_OK); + + for (i = 0; i < WORKERS; i++) { + workers[i].db = db; + workers[i].index = i; + workers[i].answer = -1; + workers[i].stage = "unstarted"; + } + for (i = 0; i < WORKERS; i++) { + if (pthread_create(&workers[i].id, NULL, count_people, &workers[i]) != 0) { + ZT_FAIL("could not start worker %d", i); + } + } + for (i = 0; i < WORKERS; i++) { + pthread_join(workers[i].id, NULL); + } + + for (i = 0; i < WORKERS; i++) { + if (workers[i].failed_at != ZU_OK) { + ZT_FAIL("worker %d stopped at %s with status %d after %d rounds", i, workers[i].stage, + (int)workers[i].failed_at, workers[i].rounds); + } + ZT_CHECK_EQ(workers[i].rounds, ROUNDS); + ZT_CHECK_EQ(workers[i].answer, PEOPLE); + } + + zu_database_close(db); +} + +/* A connection handed from one thread to the next, one at a time. */ +struct handoff { + zu_conn *conn; + int64_t answer; + zu_status status; +}; + +static void *use_the_connection_once(void *arg) { + struct handoff *h = (struct handoff *)arg; + zu_result *res = NULL; + const zu_value *cell = NULL; + h->status = zu_query_z(h->conn, "RETURN 42 AS n", &res, NULL); + if (h->status == ZU_OK) { + if (zu_result_cell(res, 0, 0, &cell) != ZU_OK || zu_value_i64(cell, &h->answer) != ZU_OK) { + h->status = ZU_MISUSE; + } + } + zu_result_free(res); + return NULL; +} + +ZT_TEST(a_connection_moves_between_threads_as_long_as_only_one_holds_it) { + /* "A connection may move between threads but must not be used from + * two at once." The second half of that sentence is the famous one + * and the first half is the one a pool depends on: a worker takes a + * connection off a queue, uses it, and puts it back for a different + * worker to take. + * + * So the connection is opened here, used on a thread, joined, used on + * another thread, joined, and closed here. Nothing shares it at any + * instant and every handoff is a join, which is the discipline a pool + * has to keep and the one TSan can check. */ + zu_conn *conn = NULL; + int i; + + ZT_CHECK_EQ(zu_memory(&conn, NULL), ZU_OK); + for (i = 0; i < 16; i++) { + struct handoff h; + pthread_t worker; + memset(&h, 0, sizeof h); + h.conn = conn; + h.status = ZU_MISUSE; + if (pthread_create(&worker, NULL, use_the_connection_once, &h) != 0) { + zu_conn_close(conn); + ZT_FAIL("could not start the worker for pass %d", i); + } + pthread_join(worker, NULL); + if (h.status != ZU_OK) { + zu_conn_close(conn); + ZT_FAIL("pass %d on its own thread answered %d", i, (int)h.status); + } + ZT_CHECK_EQ(h.answer, 42); + } + /* And it belongs to whoever holds it, including this thread, which is + * where it started. */ + { + zu_result *res = NULL; + const zu_value *cell = NULL; + int64_t n = 0; + ZT_CHECK_EQ(zu_query_z(conn, "RETURN 7 AS n", &res, NULL), ZU_OK); + ZT_CHECK_EQ(zu_result_cell(res, 0, 0, &cell), ZU_OK); + ZT_CHECK_EQ(zu_value_i64(cell, &n), ZU_OK); + ZT_CHECK_EQ(n, 7); + zu_result_free(res); + } + zu_conn_close(conn); +} + +/* One connection and two threads, which is the thing the header tells a + * host not to do. The second thread hammers the connection while the + * first runs statements on it, and every call it makes has to come back + * with a status rather than with a corrupted cache. */ +struct sharer { + zu_conn *conn; + int calls; + int refused; + int allowed; + int other; + zu_status worst; +}; + +/* The stop flag goes through a mutex rather than being a volatile int, + * because this file is compiled under TSan, where a plain flag written + * on one thread and read on another is a data race and is reported as + * one. It would be a race without TSan too; the sanitizer only makes it + * visible, and a suite that reports races cannot afford to contain + * one. */ +static pthread_mutex_t stop_lock = PTHREAD_MUTEX_INITIALIZER; +static int stop_now; + +static int should_stop(void) { + int stop; + pthread_mutex_lock(&stop_lock); + stop = stop_now; + pthread_mutex_unlock(&stop_lock); + return stop; +} + +static void set_stop(int value) { + pthread_mutex_lock(&stop_lock); + stop_now = value; + pthread_mutex_unlock(&stop_lock); +} + +static void *share_the_connection(void *arg) { + struct sharer *s = (struct sharer *)arg; + while (!should_stop()) { + zu_result *res = NULL; + zu_status st = zu_query_z(s->conn, "RETURN 1 AS one", &res, NULL); + zu_result_free(res); + s->calls++; + if (st == ZU_MISUSE_CONCURRENT) { + s->refused++; + } else if (st == ZU_OK) { + s->allowed++; + } else { + s->other++; + s->worst = st; + } + } + return NULL; +} + +ZT_TEST(one_connection_in_two_threads_is_refused_rather_than_raced) { + /* misuse.c gets this deterministically from inside the progress + * watcher, which is the version that always fires. This is the same + * rule the way a host meets it by accident: two threads genuinely + * racing, for as long as it takes the first to run five hundred + * statements. + * + * The assertion cannot be that every call was refused, because a call + * that arrives while the connection is idle is a call on an idle + * connection and is allowed. What it can be, and what the header + * promises, is that nothing comes back as any other kind of failure + * and that both threads keep getting right answers. A raced cache + * shows up as a wrong number or a crash rather than as a status. */ + zu_conn *conn = NULL; + struct sharer state; + pthread_t other; + int i; + + memset(&state, 0, sizeof state); + set_stop(0); + + ZT_CHECK_EQ(zu_memory(&conn, NULL), ZU_OK); + state.conn = conn; + + if (pthread_create(&other, NULL, share_the_connection, &state) != 0) { + zu_conn_close(conn); + ZT_FAIL("could not start the second thread"); + } + + for (i = 0; i < 500; i++) { + zu_result *res = NULL; + const zu_value *cell = NULL; + int64_t n = 0; + zu_status st = zu_query_z(conn, "RETURN 41 + 1 AS n", &res, NULL); + if (st == ZU_MISUSE_CONCURRENT) { + /* This thread lost the toss, which is allowed and is the same + * answer the other thread gets when it loses. */ + continue; + } + if (st != ZU_OK) { + set_stop(1); + pthread_join(other, NULL); + zu_conn_close(conn); + ZT_FAIL("a statement on the owning thread answered %d", (int)st); + } + if (zu_result_cell(res, 0, 0, &cell) != ZU_OK || zu_value_i64(cell, &n) != ZU_OK || n != 42) { + zu_result_free(res); + set_stop(1); + pthread_join(other, NULL); + zu_conn_close(conn); + ZT_FAIL("a statement run beside another thread answered %lld", (long long)n); + } + zu_result_free(res); + } + + set_stop(1); + pthread_join(other, NULL); + + ZT_CHECK(state.calls > 0); + /* Nothing but the two expected answers. Anything else would mean the + * intruding thread got far enough in for the engine to fail the work + * rather than turning it away at the door. */ + if (state.other != 0) { + zu_conn_close(conn); + ZT_FAIL("%d of %d calls from the second thread answered %d, which is neither ok nor concurrent", + state.other, state.calls, (int)state.worst); + } + + /* And the connection is fit to use afterwards, which is the whole + * point of refusing rather than racing. */ + { + zu_result *res = NULL; + const zu_value *cell = NULL; + int64_t n = 0; + ZT_CHECK_EQ(zu_query_z(conn, "RETURN 7 AS n", &res, NULL), ZU_OK); + ZT_CHECK_EQ(zu_result_cell(res, 0, 0, &cell), ZU_OK); + ZT_CHECK_EQ(zu_value_i64(cell, &n), ZU_OK); + ZT_CHECK_EQ(n, 7); + zu_result_free(res); + } + zu_conn_close(conn); +} + +/* The one call the header says is meant to cross threads. */ +struct interrupter { + zu_conn *conn; + zu_status status; +}; + +static void *interrupt_soon(void *arg) { + struct interrupter *state = (struct interrupter *)arg; + /* Long enough that the statement is certainly running, short enough + * that it is certainly not finished. The statement below takes about + * a third of a second uninstrumented and longer under a sanitizer, + * which is the direction it is safe to be wrong in. */ + usleep(50 * 1000); + state->status = zu_conn_interrupt(state->conn); + return NULL; +} + +ZT_TEST(an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection) { + /* zu_conn_interrupt is documented as the exception to the sharing + * rule, and the reason given is worth checking rather than trusting: + * "a cancellation that had to wait for the connection to be free + * could only arrive after the statement it was meant to stop". + * + * So the interrupt has to be accepted while the connection is busy, + * which is exactly what every other call is refused for; the + * statement has to come back ZU_INTERRUPTED rather than failing; and + * the connection has to run the next statement normally, which is + * what the header says makes this different from closing it. */ + static int64_t many[3000]; + zu_database *db = NULL; + zu_conn *conn = NULL; + zu_frame *frame = NULL; + zu_result *res = NULL; + struct interrupter state; + pthread_t other; + int i; + + for (i = 0; i < 3000; i++) { + many[i] = i; + } + memset(&state, 0, sizeof state); + + ZT_CHECK_EQ(zu_database_memory(NULL, &db, NULL), ZU_OK); + ZT_CHECK_EQ(zu_connect(db, &conn, NULL), ZU_OK); + ZT_CHECK_EQ(zu_frame_new_z("Person", 3000, NULL, NULL, &frame, NULL), ZU_OK); + ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, many, 3000, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK); + ZT_CHECK_EQ(zu_conn_register(conn, frame, NULL), ZU_OK); + + state.conn = conn; + state.status = ZU_MISUSE; + if (pthread_create(&other, NULL, interrupt_soon, &state) != 0) { + ZT_FAIL("could not start the interrupting thread"); + } + + /* Four and a half million pairs, which is long enough to be stopped + * in the middle of. */ + ZT_CHECK_EQ( + zu_query_z(conn, "MATCH (a:Person), (b:Person) WHERE a.id < b.id RETURN count(*)", &res, NULL), + ZU_INTERRUPTED); + ZT_CHECK(res == NULL); + pthread_join(other, NULL); + + /* Accepted while the connection was busy, rather than turned away the + * way every other call would have been. */ + ZT_CHECK_EQ(state.status, ZU_OK); + + /* And nothing is wrong with the connection. */ + { + const zu_value *cell = NULL; + int64_t n = 0; + ZT_CHECK_EQ(zu_query_z(conn, "RETURN 7 AS n", &res, NULL), ZU_OK); + ZT_CHECK_EQ(zu_result_cell(res, 0, 0, &cell), ZU_OK); + ZT_CHECK_EQ(zu_value_i64(cell, &n), ZU_OK); + ZT_CHECK_EQ(n, 7); + zu_result_free(res); + } + + zu_conn_close(conn); + zu_frame_free(frame); + zu_database_close(db); +} + +ZT_MAIN(ZT_CASE(eight_threads_with_a_connection_each_read_one_database_and_agree), + ZT_CASE(a_connection_moves_between_threads_as_long_as_only_one_holds_it), + ZT_CASE(one_connection_in_two_threads_is_refused_rather_than_raced), + ZT_CASE(an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection)) From 279e9e6a54e2afcc9f308ce090950e731370284d Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:30:16 +0700 Subject: [PATCH 2/2] run the threading suite under TSan and both C suites under valgrind The README promised ASan, UBSan, TSan and valgrind over the ABI. The first two were there and the last two were not, so this adds them. Valgrind is the straightforward half. Both C files run under memcheck, definitely and indirectly lost count as errors, and both are clean: zero errors, zero definitely lost, zero indirectly lost. Possibly lost is one 180 byte block belonging to a thread the library starts, which is what a thread-local arena looks like to a leak checker. TSan is not straightforward, and the honest version of it took longer than the job. libzu takes no pthread lock at all: nm shows no pthread_mutex, no pthread_rwlock, no pthread_cond, and one syscall, which is the futex Rust's std locks take directly. TSan learns about an uninstrumented library through the pthread calls it intercepts, so it has no edge to learn from and reports the engine as racing with itself. Eighteen reports came out of the eight-thread case. None of them means anything, and the way to know is to take the sharing away: eight threads on eight separate databases, sharing nothing whatever, still report one, a malloc on one thread against a free of the same address on another. So reports from inside libzu are suppressed, with the measurement written down in test/tsan.supp and filed against the engine as tamnd/zu#622, which asks for the instrumented build in which those reports could be read instead. What the job checks is the half this repository owns, and that it checks anything is itself checked: a race planted in the suite's own memory is still reported with the suppressions on, exactly as the leak probe was checked before believing LSan. Also adds ZU_TEST_ROWS, because the two fixtures that exist in order to be slow are slow enough already once memcheck is slowing the machine by forty. The valgrind job passes 300 and gets the same behaviour for the same reason, in a fifth of the wall clock. --- .github/workflows/ci.yml | 115 +++++++++++++++++++++++++++++++++++++++ README.md | 2 +- test/harness.h | 34 ++++++++++++ test/misuse.c | 11 ++-- test/threads.c | 30 +++++++--- test/tsan.supp | 35 ++++++++++++ 6 files changed, 212 insertions(+), 15 deletions(-) create mode 100644 test/tsan.supp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e56def4..594fe14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,6 +156,121 @@ jobs: ASAN_OPTIONS: detect_leaks=1 UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 + # Races, which are the failure the address sanitizer cannot see and + # the one an ABI used from four threads is most likely to have. + # + # gcc rather than clang, because the suppression file below was + # measured against gcc's libtsan and a suppression nobody checked is + # worse than no suppression at all. + tsan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - uses: actions/checkout@v5 + with: + repository: tamnd/zu + path: engine + + - uses: Swatinem/rust-cache@v2 + with: + workspaces: engine + + - uses: lukka/get-cmake@latest + + - name: Build libzu + working-directory: engine + run: cargo build --release -p zu-capi + + - name: Configure + run: > + cmake -B build-tsan -DCMAKE_BUILD_TYPE=RelWithDebInfo + -DZU_ROOT="$GITHUB_WORKSPACE/engine" + -DZU_CPP_SANITIZE=thread + env: + CC: gcc + CXX: g++ + + - name: Build the threading suite + run: cmake --build build-tsan --target threads + + # Only the threading suite, because it is the only file here that + # starts a thread. The rest of the tree under TSan would be a + # slower way of compiling code that never races. + # + # The suppression file says at length why the engine's own frames + # are dropped: libzu takes no pthread lock, so TSan has no edge to + # learn from and reports two threads as racing even when one of + # them plainly waited. What is left is this repository's half, + # and it is checked rather than assumed: a race planted in the + # suite's own memory is still reported with these suppressions on. + - name: The threading rule holds when it is kept + run: ./build-tsan/test/threads + env: + TSAN_OPTIONS: suppressions=${{ github.workspace }}/test/tsan.supp:halt_on_error=1 + + # And memcheck, which sees what the sanitizers do not: an + # uninstrumented library's own reads and writes. libzu is compiled by + # cargo without instrumentation, so ASan knows nothing about what it + # does between the malloc and the free that it intercepts, and + # valgrind knows all of it. + valgrind: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - uses: actions/checkout@v5 + with: + repository: tamnd/zu + path: engine + + - uses: Swatinem/rust-cache@v2 + with: + workspaces: engine + + - uses: lukka/get-cmake@latest + + - name: Install valgrind + run: sudo apt-get update && sudo apt-get install -y valgrind + + - name: Build libzu + working-directory: engine + run: cargo build --release -p zu-capi + + - name: Configure + run: > + cmake -B build-vg -DCMAKE_BUILD_TYPE=RelWithDebInfo + -DZU_ROOT="$GITHUB_WORKSPACE/engine" + + - name: Build the C suites + run: cmake --build build-vg --target misuse threads + + # The two C files, because they are the ones that hand raw + # pointers across and give everything back by hand. + # + # Definitely and indirectly lost are errors. Possibly lost is not: + # it is one block of 180 bytes, it belongs to a thread the library + # started, and a pointer into the middle of a block is how a + # thread-local arena looks to a leak checker rather than how a + # leak looks. + # + # ZU_TEST_ROWS shrinks the two fixtures that exist to be slow. + # Memcheck already runs the machine forty times slower, which is + # another way of getting a statement that lasts long enough to be + # interrupted, so the smaller number tests the same thing in a + # fifth of the wall clock. + - name: The C suites under memcheck + run: | + set -eu + for t in misuse threads; do + valgrind --error-exitcode=99 --leak-check=full \ + --show-leak-kinds=definite,indirect \ + --errors-for-leak-kinds=definite,indirect \ + --track-origins=yes "./build-vg/test/$t" + done + env: + ZU_TEST_ROWS: 300 + # The claim the packaging makes is that a project that installed this # can find it. Nothing in the suite can check that, because the suite # is inside the build tree and finds the header by being next to it. diff --git a/README.md b/README.md index 15ed22c..f62e1fa 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ lynn - `bench/`, the numbers below, with a timing harness that needs no package manager to run. - `cmake/`, `find_package(Zu)` to find the engine and `find_package(zu-cpp)` to find this. vcpkg, Conan and pkg-config packaging come with the first release. -Still to come: ASan, UBSan, TSan and Valgrind suites over the full ABI surface, including the deliberate misuse cases, because an ABI nine languages depend on should fail loudly rather than corrupt quietly. +Four sanitizer jobs run over the suite, because an ABI nine languages depend on should fail loudly rather than corrupt quietly. The whole tree runs under ASan and UBSan; `test/misuse.c` runs again with leak detection on, which it can and the C++ files cannot, because it is the file that gives every handle back by hand; `test/threads.c` runs under TSan; and both C files run under valgrind, which sees what the sanitizers cannot, since libzu is compiled without instrumentation and memcheck does not need any. `test/tsan.supp` records what TSan is unable to be told about a library that takes no pthread lock, and why the reports from inside the engine are dropped rather than read. ## Building diff --git a/test/harness.h b/test/harness.h index f0c5866..85ba4dc 100644 --- a/test/harness.h +++ b/test/harness.h @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -229,6 +230,39 @@ static inline int zt_next_fd(void) { return fd; } +/* How many rows a fixture that exists in order to be slow should hold. + * + * Two cases here need a statement that is still running when something + * else happens to it: the progress watcher has to fire while it runs, + * and the interrupt has to land before it ends. Both get that by + * counting pairs over three thousand people, which takes about a third + * of a second and is nothing. + * + * Under valgrind it is not nothing. Memcheck runs somewhere between + * twenty and fifty times slower, and a third of a second becomes ten + * seconds or more of a job whose other cases finish instantly. So the + * valgrind runs pass a smaller number, and get the same behaviour for + * the same reason: what those two cases need is a statement that lasts + * long enough to be interrupted, and slowing the machine down by forty + * is another way of arriving at one. + * + * The default is the whole number, so nothing changes for anyone who + * does not set it, and the value is clamped at the whole number so a + * larger one cannot overrun the array the caller sized. */ +static inline uint64_t zt_rows(uint64_t whole) { + const char *set = getenv("ZU_TEST_ROWS"); + unsigned long asked; + char *end = NULL; + if (set == NULL || *set == '\0') { + return whole; + } + asked = strtoul(set, &end, 10); + if (end == set || asked == 0) { + return whole; + } + return (uint64_t)asked > whole ? whole : (uint64_t)asked; +} + typedef struct zt_case { const char *name; void (*fn)(void); diff --git a/test/misuse.c b/test/misuse.c index da71325..7a78bf5 100644 --- a/test/misuse.c +++ b/test/misuse.c @@ -570,22 +570,23 @@ ZT_TEST(a_call_back_into_the_library_from_the_watcher_is_refused_rather_than_rac * call made from inside it is a call made from a second thread at a * moment when the first is certainly inside the executor. */ static int64_t ids[3000]; + const uint64_t rows = zt_rows(3000); zu_database *db = NULL; zu_conn *conn = NULL; zu_frame *frame = NULL; zu_result *res = NULL; struct reentry state; - int i = 0; + uint64_t i = 0; - for (i = 0; i < 3000; i++) { - ids[i] = i; + for (i = 0; i < rows; i++) { + ids[i] = (int64_t)i; } memset(&state, 0, sizeof state); ZT_CHECK_EQ(zu_database_memory(NULL, &db, NULL), ZU_OK); ZT_CHECK_EQ(zu_connect(db, &conn, NULL), ZU_OK); - ZT_CHECK_EQ(zu_frame_new_z("Person", 3000, NULL, NULL, &frame, NULL), ZU_OK); - ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, ids, 3000, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK); + ZT_CHECK_EQ(zu_frame_new_z("Person", rows, NULL, NULL, &frame, NULL), ZU_OK); + ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, ids, rows, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK); ZT_CHECK_EQ(zu_conn_register(conn, frame, NULL), ZU_OK); state.conn = conn; diff --git a/test/threads.c b/test/threads.c index 32c82fd..c4c8813 100644 --- a/test/threads.c +++ b/test/threads.c @@ -28,11 +28,11 @@ */ #include +#include #include #include #include #include -#include #include "harness.h" @@ -381,13 +381,23 @@ struct interrupter { zu_status status; }; +/* Waiting on nothing for a while, which is the sleep that needs no + * feature test macro. usleep was withdrawn from POSIX in 2008 and glibc + * hides it from a file compiled at -std=c11; nanosleep is there but its + * declaration is behind __USE_POSIX199309, so reaching it means + * defining _POSIX_C_SOURCE at the top of the file, which changes what + * every other header here declares and does so differently on macOS + * than on glibc. poll is declared by poll.h on both without asking for + * anything, and a poll of no descriptors is a timer. */ +static void sleep_ms(int ms) { poll(NULL, 0, ms); } + static void *interrupt_soon(void *arg) { struct interrupter *state = (struct interrupter *)arg; /* Long enough that the statement is certainly running, short enough * that it is certainly not finished. The statement below takes about * a third of a second uninstrumented and longer under a sanitizer, * which is the direction it is safe to be wrong in. */ - usleep(50 * 1000); + sleep_ms(50); state->status = zu_conn_interrupt(state->conn); return NULL; } @@ -404,23 +414,24 @@ ZT_TEST(an_interrupt_from_another_thread_stops_the_statement_and_not_the_connect * the connection has to run the next statement normally, which is * what the header says makes this different from closing it. */ static int64_t many[3000]; + const uint64_t rows = zt_rows(3000); zu_database *db = NULL; zu_conn *conn = NULL; zu_frame *frame = NULL; zu_result *res = NULL; struct interrupter state; pthread_t other; - int i; + uint64_t i; - for (i = 0; i < 3000; i++) { - many[i] = i; + for (i = 0; i < rows; i++) { + many[i] = (int64_t)i; } memset(&state, 0, sizeof state); ZT_CHECK_EQ(zu_database_memory(NULL, &db, NULL), ZU_OK); ZT_CHECK_EQ(zu_connect(db, &conn, NULL), ZU_OK); - ZT_CHECK_EQ(zu_frame_new_z("Person", 3000, NULL, NULL, &frame, NULL), ZU_OK); - ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, many, 3000, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK); + ZT_CHECK_EQ(zu_frame_new_z("Person", rows, NULL, NULL, &frame, NULL), ZU_OK); + ZT_CHECK_EQ(zu_frame_col_int(frame, "id", 2, many, rows, 64, 1, 1, ZU_FRAME_PLAIN, NULL), ZU_OK); ZT_CHECK_EQ(zu_conn_register(conn, frame, NULL), ZU_OK); state.conn = conn; @@ -429,8 +440,9 @@ ZT_TEST(an_interrupt_from_another_thread_stops_the_statement_and_not_the_connect ZT_FAIL("could not start the interrupting thread"); } - /* Four and a half million pairs, which is long enough to be stopped - * in the middle of. */ + /* Every pair of people with a predicate over them, which the planner + * cannot fold into a count, so it runs long enough to be stopped in + * the middle of. */ ZT_CHECK_EQ( zu_query_z(conn, "MATCH (a:Person), (b:Person) WHERE a.id < b.id RETURN count(*)", &res, NULL), ZU_INTERRUPTED); diff --git a/test/tsan.supp b/test/tsan.supp new file mode 100644 index 0000000..f93a35f --- /dev/null +++ b/test/tsan.supp @@ -0,0 +1,35 @@ +# What the thread sanitizer cannot be told about libzu. +# +# TSan works out what happened before what by watching synchronization, +# and the way it watches an uninstrumented library is by intercepting +# the pthread calls it makes. libzu makes none. The measurement: +# +# $ nm -D libzu.so | grep -E 'U (pthread_mutex|pthread_rwlock|pthread_cond)' +# $ nm -D libzu.so | grep -E 'U (syscall|futex)' +# U syscall@GLIBC_2.2.5 +# +# Not one pthread primitive, and one syscall, which is the futex Rust's +# own locks take directly rather than through libc. So every lock the +# engine holds is invisible, every happens-before edge it establishes is +# lost, and any two threads that touch the same byte look to TSan like +# two threads racing on it even when one of them plainly waited. +# +# That is not a guess. Eight threads on eight separate databases, +# sharing nothing whatever, still report one: +# +# Write of size 8 by thread T4: #0 free #1 libzu.so+0x1f3e00 +# Previous write of size 8 by T1: #0 malloc #1 libzu.so+0x1f7167 +# +# which is an allocator handing one thread a block another gave back. +# If a report can be raised by two threads that share nothing, a report +# raised by two threads that share a database says nothing either. +# +# So the reports are suppressed rather than read, and what the job +# checks is the half of the boundary this repository owns: that the +# suite's own threading is sound, that a connection handed between +# threads is handed with a real edge, and that the library is not being +# entered from two threads at once by accident. A race inside the engine +# is the engine's to find, with an instrumented build of its own, which +# is tamnd/zu#622. +called_from_lib:libzu.so +race:libzu.so