Skip to content

Closing a connection does not give back everything it took, after the connection has been interrupted or shared #778

Description

@tamnd

zu-c runs its two C suites under valgrind on every push, and the threads suite comes back with 5,184 bytes definitely lost in 5 blocks and 14,349 indirectly lost in 126 more. Every one of the five roots is an allocation the engine made, and every one of them is behind a call the test does make and then does close.

The run is against libzu.so from cargo build --release -p zu-capi on main, on ubuntu-latest, under valgrind --leak-check=full --show-leak-kinds=definite,indirect --track-origins=yes. Frames inside the library print as ??? because the release profile carries -C strip=symbols; the exported entry points still resolve, which is enough to say where each block came in.

The five roots:

114 (88 direct, 26 indirect) bytes in 1 blocks are definitely lost in loss record 115 of 169
   at 0x4846828: malloc
   by 0x4A28399: zu_database_memory (libzu.so)
   by 0x10A18B: an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection (threads.c:431)

528 (80 direct, 448 indirect) bytes in 1 blocks are definitely lost in loss record 146 of 169
   at 0x4846828: malloc
   by 0x4A2D310: zu_frame_new (libzu.so)
   by 0x4A2D837: zu_frame_new_z (libzu.so)
   by 0x10A1D1: an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection (threads.c:433)

1,063 (392 direct, 671 indirect) bytes in 1 blocks are definitely lost in loss record 152 of 169
   at 0x4846828: malloc
   by 0x4A3601D: zu_query (libzu.so)
   by 0x10A282: an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection (threads.c:446)

7,531 (2,312 direct, 5,219 indirect) bytes in 1 blocks are definitely lost in loss record 166 of 169
   at 0x4846828: malloc
   by 0x4A33EAF: zu_memory (libzu.so)
   by 0x10A675: one_connection_in_two_threads_is_refused_rather_than_raced (threads.c:316)

10,297 (2,312 direct, 7,985 indirect) bytes in 1 blocks are definitely lost in loss record 167 of 169
   at 0x4846828: malloc
   by 0x4A26578: zu_connect (libzu.so)
   by 0x10A1A8: an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection (threads.c:432)

What the two tests do with those handles. an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection opens a database in memory, connects, registers a frame, starts a second thread that calls zu_conn_interrupt after fifty milliseconds, runs a statement that comes back ZU_INTERRUPTED with a null result, joins the thread, runs RETURN 7 on the same connection and gets 7, and then calls zu_conn_close, zu_frame_free and zu_database_close in that order. one_connection_in_two_threads_is_refused_rather_than_raced calls zu_memory, starts a second thread that hammers the same connection while the first runs five hundred statements on it, joins, runs one more statement, and calls zu_conn_close. Both give everything back, in the order the header asks for, and neither leaves a result unfreed: the interrupted zu_query answered a null result, which is what the header says it does.

The interesting part is the contrast. The misuse suite, which is the other half of the same job, opens and closes hundreds of connections and databases the same way and comes back with definitely lost: 0 bytes in 0 blocks and ERROR SUMMARY: 0 errors from 0 contexts. So it is not that closing a connection leaks. It is that closing a connection leaks after something has happened to it, and the two things that happened here are an interrupt and a second thread being turned away with ZU_MISUSE_CONCURRENT.

Three guesses, offered as guesses. The interrupted statement may leave the plan or the executor state owned by nothing once the interrupt unwinds it, in which case the zu_query root is the leak and the zu_connect root is the connection still holding it. A refused concurrent call may leave the guard in a state that makes zu_conn_close decline to tear down rather than tear down, which would explain a whole connection surviving in the second test. Or the teardown path may be skipped entirely when the connection was last touched by a thread other than the one closing it.

The frames would say which, and they would say it in one reading, but they need a build with CARGO_PROFILE_RELEASE_STRIP=none to have any names in them. That is one line in the reproduction and it is worth the rebuild.

Reproducing it, from a checkout of tamnd/zu-c beside a checkout of this repository:

cd engine && CARGO_PROFILE_RELEASE_STRIP=none cargo build --release -p zu-capi && cd ..
cmake -B build-vg -DCMAKE_BUILD_TYPE=RelWithDebInfo -DZU_ROOT="$PWD/engine"
cmake --build build-vg --target threads
valgrind --leak-check=full --show-leak-kinds=definite,indirect ./build-vg/test/threads

zu-c is suppressing these five roots by name for now, in test/memcheck.supp, so that the job can go green over the rest of what it checks. The suppressions name this issue and come out when it closes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions