Repository navigation
test: gate the mimalloc v2 pin off the manifests — and close out #297, which was #294 all along - #447
Merged
Merged
Conversation
…ow cannot hide it #297 recorded tests/test_docs_snippets.py as never terminating when selected on its own — CPU-bound, no output — and reasoned it was a separate ordering bug because a spin is not corruption. It was #294: two co-resident mimalloc instances handing out overlapping memory. Reconstructed as a 2x2 against a worktree at the commit the issue names, using the module's own selection as the probe: v3 wheel + pyarrow 25.0.0 -> HUNG, state R, CPU tracking elapsed v2 wheel + pyarrow 25.0.0 -> passed v3 wheel + pyarrow 25.0.1 -> passed v2 wheel + pyarrow 25.0.1 -> passed (today's environment, 20/20) Either fix alone clears it, and both are in place — which is why the issue's reproduction no longer reproduces. A corrupted free list can spin as readily as it can crash, and which one surfaces depends on allocation order: that is the whole of the "fine with its siblings, not on its own" behaviour. What that leaves is a gap. The runtime guard says of itself that it can only go red on pyarrow 24.0.0-25.0.0, and the lock has been past that window since 25.0.1. So nothing currently watches the pin — the half we control, and the half that protects a user whose pyarrow we do not choose. This reads it off the manifests instead, on any pyarrow, discovering the crates by their #[global_allocator] rather than listing them so a new surface is covered the day it is added. Buildless, so it runs in repo-gates. Falsified by dropping the feature from laterite-node: red, and green again on restore. The wiki page that owns this fault now records the second face, because a fault you triage by symptom is one you re-file.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
✅ Based on
main— no stacking, safe to merge when CI is green.Started as "fix #297" and finished somewhere else: the hang no longer reproduces, because it was never its own bug.
What #297 actually was
#297 recorded
tests/test_docs_snippets.pyas never terminating when selected on its own — CPU-bound, no output, no diagnostic — and argued it was not #294, on the grounds that corruption and a spin are different shapes. Reasonable at the time, and wrong: it is the same co-resident-allocator fault.Reconstructed against a worktree at the commit the issue names (
42262c3), using the module's own selection as the probe. Two variables, four cells:R, CPU tracking elapsed, killed at 55sThe lock at
42262c3had pyarrow 25.0.0, and the v2 pin (#301) landed after the issue was filed, so both halves of the fault were present then and neither is now. Either fix alone clears it.The "narrow selection versus whole directory" behaviour that made it look like an ordering bug falls out of the mechanism: a corrupted free list can spin as readily as it can crash, and which one you get depends on allocation order. Collecting the siblings changes the allocation order.
What this PR adds, and why it is not "nothing to do"
Both halves are fixed, so there is no hang to fix. There is a gap, though, and it is the interesting part.
The runtime guard (
test_pyarrow_buffers_survive_a_coresident_native_module) says of itself, in its own comment, that it can only go red on pyarrow 24.0.0–25.0.0. The lock has been past that window since 25.0.1. So today that guard passes no matter what the pin says — the pin being the half we control, and the half that protects a user whose pyarrow we do not choose.tests/test_allocator_pin_faithful.pyreads the pin off the manifests instead:#[global_allocator]can only be set by a final artifact, so a new binding or binary is covered the day it is added, not the day someone remembers this file;features = ["v2"]on each one's mimalloc dep, with the reason in the failure message (omitting the feature is not a neutral default —libmimalloc-sysresolves v3 from 0.1.47 on, so the faulting configuration is three deleted characters away);repo-gatesin about a tenth of a second rather than behind a release build.Falsified by dropping the feature from
laterite-node: red on that crate alone, green again on restore.Wiki
concepts/arrow-c-ffi-allocator-ownership.mdowns this fault and did not know it had a second face. It does now, with the instruction that matters more than the anecdote: triage this one by its precondition (two co-resident allocators), never by its symptom. A fault you triage by symptom is one you re-file.Follow-up
#297 can close — recommend the maintainer does so; the evidence is posted there. Nothing in it needs fixing, and its "Not the same bug as #294" section is superseded by the table above.