Misuse and lifecycle, watched by a sanitizer and a leak checker - #19
Merged
Conversation
DX3 asks for a misuse suite and a lifecycle suite in every client. The misuse half has been here since DX2; this is the other one, and the two defects it turned up. A parameter that contains itself was a segfault. The conversion walked the value recursively, so a dict holding itself ran the C stack out and took the interpreter with it, which arrives as a signal rather than as a RecursionError because the recursion is on this side of the boundary. It is refused at depth 64 now, where a real value never reaches and a cycle always does, as a ValueError, which is what the standard library raises for a circular reference. A row appended through a connection that was closed under the appender was accepted and buffered. The flush was where it was noticed, so the same call was refused or not depending on whether the batch happened to fill, which is a rule nobody can hold in their head. The connection is checked where the appender is, at the call that made the mistake. The lifecycle tests are three: a thousand connections opened and closed, a thousand opened and dropped for the callers who never write close, and one closed with an appender and a transaction still open on it. Each counts the process descriptors from outside rather than trusting the client to report on itself.
A sanitizer job and a leak job over the same suite, because they see different things. ASan instruments the source it compiles and catches what this extension does to its own memory; valgrind instruments the instructions and catches both sides of the boundary, every prebuilt thing either of them links, and a read of memory nobody wrote, which ASan does not look for at all. Both deselect the timing marker, which the tests that assert on a wall clock now carry. A ratio between two things a sanitizer slowed down by different factors is not a measurement of anything. The leak job installs pandas rather than everything, because polars starts a thread pool on import and holds thread-local state for the life of the process, and none of it belongs here. tools/valgrind.supp names the interpreter as the allocator rather than listing symptoms, so it cannot grow a line every bad week, and it says out loud what that leaves out.
Griffe reads the package the way a reader does, out of the .py files and the .pyi that declares the compiled half, and it names what moved, changed shape or went away. Nothing is built in this job on purpose: the declared API is what the stub says, so the stub is what gets compared, and a stub that has drifted from the extension is what the typing tests in the suite are for. A break is allowed and has to be paid for with a version bump, which is the rule cargo semver-checks applies to the engine and the rule api-extractor's committed report applies to the TypeScript client. The change is fine; the change happening quietly is not. The gate is validated the way the leak gate beside it is: renaming a class the DB-API layer inherits from has to fail it, and the tree goes back before the real check reads it.
Two things the first run of these jobs found. The sanitizer job had no pandas, so the two whole programs the README publishes that call to_pandas failed inside it and the job was watching a smaller suite than it claimed to. They are installed by name rather than through the extra that names them, because the extra would build this extension from source to read its metadata and this job builds its own a step later. The leak job counted possible losses as well as definite ones, and over the whole suite that is 420 possible and zero definite, of which 417 are pyarrow registering compute kernels into a static table it never tears down. A possible loss is a block whose only surviving pointer is into its middle, which is what a registry of C++ objects looks like from outside and what almost nothing this extension allocates looks like. The alternative was a suppression naming pyarrow, and naming a dependency is how a suppression file starts growing. This is the same pair of flags the TypeScript client's job carries.
The leak job was red on two counts, and neither of them was a leak. Three tests assert wall clock. An empty transaction under 50 us, ten thousand turns of the main thread while a result becomes Arrow, twenty turns of the event loop while a statement runs. Under valgrind every instruction is interpreted and the machine is between twenty and fifty times slower, so all three fail and say so in microseconds, which reads like a regression and is not one. They carry the timing marker now, which is what that marker is for and what the job's own -m "not timing" was already asking for. The other count is the suppression file, which was letting through twenty two records that belong to nobody here. Seventeen are readline building its keymaps and terminfo strings at import and never freeing them, which pytest pays for because pytest imports readline. One is glibc keeping the old loader scope array after a dlopen grows it, which it does on purpose so a thread walking the list does not have it freed underneath. One is pyarrow caching a tzinfo. The rest are the interpreter allocating for itself, and those should already have been covered: the rule said obj:*/python3*, and the interpreter on the runner is a shared build whose code lives in libpython3.14.so, which that pattern does not name. Fixing that turned up the more interesting half. A * in a valgrind object pattern matches slashes, so */python3* also matched every extension under lib/python3.14/site-packages, this one included. The rule meant to name the interpreter was covering the code the job exists to watch. It now names the two files an interpreter can be, the shared library and the static binary, and nothing else. The three library rules are narrow and each says what leaks, how much, and why the library is right to do it. The gate step is unchanged and still fires on its deliberate leak, which is what says the file did not grow into a way of making the build green.
tamnd
force-pushed
the
misuse-lifecycle
branch
from
August 19, 2026 01:53
c0a0e48 to
7b17f45
Compare
Both Windows rows have been red for a while, and the log said almost nothing: five lines of dots, then an exit code, no failure line and no summary. That shape is not a test failing. It is the interpreter not being there to write the summary. os.kill(os.getpid(), signal.SIGINT) is what this file used to press the key, and on Windows os.kill is not a signal at all. CPython special cases exactly two values there, CTRL_C_EVENT and CTRL_BREAK_EVENT, and everything else falls through to OpenProcess and TerminateProcess. So the four tests that press the key were terminating the process they were running in, one of them at a time, and pytest died in the middle of the first one. The two console events are no better. A console event goes to every process attached to the console, which on a build machine is the build. What is left is the thing CPython itself calls once a real press has arrived: PyErr_SetInterrupt, spelled _thread.interrupt_main. It trips the same flag the console handler trips, so everything downstream of it is the same code on both platforms, which is all of the code this repository wrote. A tripped SIGINT, a statement deep in the executor, and the next PyErr_CheckSignals turning it into a KeyboardInterrupt on the main thread inside the fifty millisecond budget. The hop that is no longer covered on Windows is the one from the operating system into the C runtime, and nothing running inside this process can cover it without taking the process with it, which is what was happening. The helper says so where a reader will find it.
With the suite no longer terminating itself on Windows, the rows came
back with one failure on them, and it is a real one that has been there
the whole time behind the crash.
%gql --read-only C:\data\social.zu1
opens nothing. The magic lexes its line with shlex.split, which is
POSIX by default, and POSIX means a backslash is an escape. So that
path arrives at connect as C:datasocial.zu1 and the person reads that
the system cannot find a file they are looking at in the directory
listing in front of them.
The line still has to be lexed rather than split on whitespace, because
a notebook on any platform can be pointed at a path with a space in it
and quoting is how somebody says so. What changes is the escape
character, which is now the platform's: none on Windows, backslash
everywhere else, so a Unix user goes on writing two\ words and a
Windows user goes on writing what every other program on their machine
accepts. The comment character goes off for the same reason shlex.split
turns it off, which is that # is a legal character in a file name.
Four tests. A quoted path with a space in it, which runs everywhere and
is the only reason any of this is lexed at all. The Windows separators,
which runs on Windows. The Unix escape, which runs everywhere else and
is there so this cannot be fixed by taking something away. And a hash
in a name.
Fixing the object patterns uncovered what they had been hiding: one definite loss of eighty bytes inside this extension, on every run, allocated while the module was still being executed. It is pyo3's. A class with members gets its tp_members as a Vec whose pointer is handed to CPython, and CPython holds that array for as long as the type exists, so pyo3 leaks it rather than free memory the interpreter is still reading. One class here has members and the rest have getters, which is why it is one record and not eight. It is in this object file only because pyo3 is compiled into it. Naming that in the suppression file takes a symbol, and the wheel a release ships is stripped, which is why the report from CI read ??? on every frame. So the leaks job now builds with the symbol table left on. The instructions are the same optimised ones, because a build valgrind reads has to be the build that runs; what changes is that a report from this job says where, which is the difference between a leak checker and a number.
The job asked cargo to keep the symbols and got a stripped extension anyway, because two things strip this wheel and only one of them is cargo. `strip = true` in pyproject.toml is maturin's, applied to the artifact after the build rather than through the profile, so a profile that says to keep the symbols is a profile maturin then undoes. The first full run said so: 555 passed in thirty minutes, then one definite loss of eighty bytes with ??? on every frame. That record is pyo3 handing a type its member table, which the suppression file has named since it was written and could not match, because a rule that names a function cannot match a frame that has no name. MATURIN_STRIP is the setting that reaches the tool doing the stripping. The cargo pair stays: it is what puts the debug info there in the first place, and maturin only decides whether it survives. And a step that says so before valgrind runs. The failure this had was an hour of work followed by a report nobody could act on, and the check that would have caught it in a second is a look at the file for the section valgrind reads a frame's name out of.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The suite this client was missing, and the two jobs that make it mean something.
The misuse tests are the ones nobody writes: a connection used after close, a result iterated twice, an appender flushed after its connection went away, a value nested deeper than the binding will follow. Writing them found a segfault, which is in here fixed: a recursion with no depth limit on the way in, now cut off at 64, and a write attempted through a read-only connection now refused by name instead of taken.
The two jobs see different things and that is why there are two. ASan instruments the source it compiles, so it catches what the extension does to its own memory and nothing about what the interpreter does on its behalf. Valgrind instruments the instructions that run, so it watches both sides of the boundary and every prebuilt thing either side links, and it reports a read of memory nobody wrote, which ASan does not look for at all.
Three things about the recipes worth knowing before changing them.
PYTHONMALLOC=mallocis what makes the valgrind output readable, because CPython otherwise pools small objects behind its own allocator and every allocation the extension makes arrives as one 256KB arena with nothing inside it attributable to anybody. The leak job installs the pandas extra rather than all of them, because polars starts a thread pool on import and holds thread-local state per worker for the life of the process, and the one test that wants polars skips itself. And the tests that assert on a wall clock now carry atimingmarker that both jobs deselect, because a ratio between two things a sanitizer slowed down by different factors measures the sanitizer.tools/valgrind.suppnames the interpreter binary as the allocator instead of listing the symptoms it produces, so it cannot grow a line every time somebody wants the build green. The gap that leaves is written into the file: a Python object this extension leaks was allocated by the interpreter on its behalf and lands under the rule, and that is the leak the suite catches from the other side by counting live connections after a collection and process descriptors across a thousand cycles.The leak gate is validated the only way a gate can be, with a deliberate leak through ctypes that has to fail it.
584 tests pass locally, 8 deselected under the marker.