fix(pkg-py): pin pyarrow<25.0.0 to avoid a mimalloc segfault behind Streamlit e2e flakiness#268
Merged
Merged
Conversation
The Streamlit e2e fixtures (app_04_streamlit, app_09_streamlit_custom) were module-scoped: one Streamlit subprocess served every test in the class, with each test doing a fresh page.goto() against it. Observed in CI and reproduced locally (including on a commit that predates any related application changes, via an isolated git worktree): after the first test's browser session completed successfully, the server stopped re-running its script for any later session -- the static page shell still loaded, but no reactive content (e.g. the chat greeting) ever streamed in, with no error logged on either side. The condition was permanent for that server process; a fresh browser context/page didn't recover it, only a new server did (confirmed by instrumenting the render path: it ran exactly once across the whole affected test class, including pytest-rerunfailures' automatic reruns). Other e2e fixtures in this file (Shiny, Gradio, Dash) use the same module-scoped-server / fresh-page-per-test pattern without hitting this -- many sequential sessions against one server work fine for those apps in the same CI run, including the *other* Streamlit example (09-streamlit-custom-app.py, which doesn't wait on the chat message in its setup and so never surfaced this). So this isn't a blanket "Streamlit can't handle repeat sessions" issue; the actual trigger wasn't pinned down. Hypothetical fix: give each Streamlit test its own server process (function-scoped fixture) instead of sharing one across a class, trading some subprocess-startup overhead for not accumulating whatever state causes this. Validated against the reported failure locally -- it resolves the hang. Not fully validated: with fresh servers per test, a different set of tests (the ones that submit a chat message and wait on a real LLM response) showed new flakiness in local runs, plausibly from more concurrent OpenAI calls in a shorter window; that wasn't investigated further here.
The e2e hang/failure symptom ("SQL never updates", no exception
anywhere) was a Streamlit subprocess segfault, invisible because its
stdout/stderr are redirected to DEVNULL. Confirmed with
PYTHONFAULTHANDLER and traced to two compounding issues:
1. The Streamlit example apps reconstructed QueryChat (and its DuckDB
connection + polars data registration) on every script rerun.
Streamlit's ScriptRunner runs each rerun in a fresh OS thread, so
this repeatedly touched polars'/DuckDB's native code from new
threads.
2. pyarrow==25.0.0 (and polars' own Arrow conversion) segfaults when
its native conversion code is invoked from more than ~2 distinct
threads over a process's lifetime -- reproduced in isolation with
no Streamlit or querychat code involved, bisected away by pinning
pyarrow==21.0.0 / polars==1.20.0.
Fixes:
- Pin pyarrow/polars to versions that don't exhibit the segfault.
- Cache QueryChat construction in both Streamlit examples via
st.cache_resource so it's built once instead of on every rerun.
- DataFrameSource/DuckDBExecutor/PinSource now register a materialized
pyarrow.Table with DuckDB instead of the raw polars object, and open
a fresh short-lived connection per query instead of holding one
across threads -- defense in depth against the same crash class.
cpsievert
changed the title
Bisected the pyarrow segfault (see previous commit) to a regression introduced in pyarrow 25.0.0 specifically -- 21.0.0 through 24.0.0 are all fine when their native conversion code is invoked from many OS threads. polars needs no pin at all; the crash was entirely in pyarrow's own code. Replace the exact `pyarrow==21.0.0`/`polars==1.20.0` pins with `pyarrow<25.0.0` (polars left unconstrained), and add the same bound to the public `polars` extra in [project.optional-dependencies] -- the exact pins only lived in the dev/test dependency group, so real users installing `querychat[polars]` were still exposed to the same segfault in their own Streamlit apps.
Verified across multiple full e2e-suite runs that the pyarrow<25.0.0 pin alone is sufficient -- every segfault traced back to this single regression. The st.cache_resource caching in the Streamlit examples and the DataFrameSource/DuckDBExecutor/PinSource rewrite to materialize pyarrow.Table and use short-lived connections were not fixing anything the pin doesn't already fix, so revert them to keep this release-blocking fix as small as possible.
cpsievert
changed the title
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.
Why
While addressing review feedback on #267, CI's e2e suite failed on
test_04_streamlit_apps.pywithnet::ERR_CONNECTION_REFUSED. I confirmed it's unrelated to that PR by reproducing the exact same failure in an isolatedgit worktreechecked out at the commit right before any of that work — a pre-existing bug in the test suite, not caused by application code.An earlier commit on this PR fixed the immediate symptom (module-scoped Streamlit fixtures) but traded it for a different failure mode: with a fresh server per test, the tests that submit a chat message and wait on a real LLM response started timing out with no error anywhere.
What was actually happening
The "SQL never updates, no exception anywhere" symptom is a segfault in the Streamlit subprocess, invisible because its stdout/stderr are redirected to
DEVNULLin the test fixtures. Reproduced locally (macOS arm64) withPYTHONFAULTHANDLER=1and bisected topyarrow==25.0.0specifically — 21.0.0 through 24.0.0 are clean, only 25.0.0 crashes, confirmed with a ~15-line repro script that has no Streamlit or querychat code at all (justduckdb+polars/pyarrow, called from a few freshthreading.Threads).This matches a currently-open, actively-discussed upstream issue: apache/arrow#50428 (cross-filed at microsoft/mimalloc#1327), filed two days before this investigation. Both
polarsandpyarrowbundle their own copy of themimallocallocator (a common pattern for Rust-based Python extensions); when two independently-linked mimalloc instances coexist in one process with mismatched versions/heap layouts, macOS's limited TLS slots let one instance's thread-heap bookkeeping get corrupted by the other. An Arrow maintainer confirms pyarrow 25.0.0 changed its bundled mimalloc version (to 3.3.1), which shifts which other extension's mimalloc version it collides with — explaining why our bisection boundary (21–24 good, 25 bad) is the mirror image of that issue's (22/23 good, 24 bad): the crash depends on which two mimalloc versions happen to collide, not a monotonic "newer pyarrow is more broken" trend.Important caveat on severity: the upstream issue's crash is at process teardown; ours happens mid-execution, across many short-lived OS threads doing Arrow conversions (matching Streamlit's fresh-thread-per-script-rerun architecture). The issue's own first comment notes the same corruption "also fires mid-operation rather than at exit... under heavy allocation," which is consistent with what we're seeing, but I haven't proven it's the identical bug rather than the same bug class. I also have not confirmed this reproduces on Linux (our CI runners) — every detail in the upstream reports centers on Apple's specific TLS slot limits, and my one attempt to check via an x86_64 Docker/QEMU container was inconclusive (it crashed even harder, in a way that looks like a QEMU/AVX emulation artifact rather than this bug). So this fix is confirmed to resolve a real, reproducible local crash, but not yet confirmed to be the same thing failing on GitHub Actions' Linux runners — that's what this PR's CI run will tell us.
The fix
Minimal, since this is release-blocking: add
pyarrow<25.0.0to the publicpolarsextra inpyproject.toml([project.optional-dependencies]), not just the dev/test group — real users installingquerychat[polars]are exposed to the same crash risk in their own Streamlit apps, not just our test suite.polarsitself needs no pin; every crash traced back to pyarrow specifically.I initially over-fixed this (rewrote
DataFrameSource/DuckDBExecutor/PinSourceto materialize apyarrow.Tableup front and use short-lived per-query DuckDB connections, plus addedst.cache_resourcecaching to the Streamlit examples). Verified across multiple full e2e-suite runs that none of that is necessary — the version pin alone is sufficient — so reverted all of it to keep the diff minimal.st.cache_resourcein the examples is still a good practice independently, just not needed for this fix.If pyarrow doesn't resolve this cleanly upstream
apache/arrow#50428/microsoft/mimalloc#1327— an Arrow maintainer is already discussing enablingMI_TLS_MODEL_LOCALfor Apple builds as a real fix.DataFrameSourcematerialize apyarrow.Tableonce and use short-lived per-query DuckDB connections instead of one long-lived connection reused across threads — real defense-in-depth against this class of bug, but adds complexity and, for pure-pandas installs, a new hard dependency onpyarrowthat isn't otherwise required. Only worth reintroducing if the pin approach proves insufficient.Test plan
faulthandler+ fresh threads); found the matching upstream issue-n auto --reruns 2matching CI's invocationruff/pyrightclean