Skip to content

FIX: Remove tracemalloc.stop() from memory leak tests#125

Open
wtn wants to merge 1 commit intodatabento:devfrom
wtn:leak
Open

FIX: Remove tracemalloc.stop() from memory leak tests#125
wtn wants to merge 1 commit intodatabento:devfrom
wtn:leak

Conversation

@wtn
Copy link
Copy Markdown
Contributor

@wtn wtn commented Apr 18, 2026

The CI fails intermittently on memory leak tests, such as here.

The failing memory leak tests called tracemalloc.stop() inside Python::attach. Under concurrent test execution this races with tracemalloc_raw_alloc in other threads, causing a SIGSEGV. Removing the calls fixes the crash without affecting the measurements.

Replication details

The faulting thread was case_1_csv_mbo. Its stack was:

traceback_new          ← null deref at 0x8
tracemalloc_add_trace
tracemalloc_raw_alloc
new_threadstate
PyGILState_Ensure
Python::attach
PyFileLike::write      ← encode.rs:83
BufWriter::flush_buf
csv::Writer::drop      ← Drop impl flushing on scope exit
...
test_from_test_data_file::case_1_csv_mbo

The CSV encoder's Drop triggered a Python::attach, which called PyGILState_Ensurenew_threadstatetracemalloc_raw_alloc. That allocator hook tried to call traceback_new, which dereferenced a pointer at offset 8 from null, because tracemalloc.stop() in the memory leak test had already freed the internal traceback table.

I've replicated locally on macOS with the following looped command:

for i in $(seq 1 20); do
  echo "=== Run $i ===" && \
  PYO3_PYTHON=/opt/homebrew/opt/python@3.10/bin/python3.10 \
    /usr/sbin/taskpolicy -c maintenance cargo test --all-features -- --test-threads=3 \
    || { echo "FAILED on run $i"; break; }
done

Note: It triggers readily on older slow systems but is harder to trigger on newer fast systems. When I did trigger it on a current device, it was while running multiple instances simultaneously.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code builds locally with no new warnings (scripts/build.sh)
  • My code follows the style guidelines (scripts/lint.sh and scripts/format.sh)
  • New and existing unit tests pass locally with my changes (scripts/test.sh)
  • I have made corresponding changes to the documentation (CHANGELOG)
  • I have added tests that prove my fix is effective or that my feature works (not applicable)

Declaration

I confirm this contribution is made under an Apache 2.0 license and that I have the authority
necessary to make this contribution on behalf of its copyright owner.

@wtn wtn marked this pull request as ready for review April 18, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant