fix(windows): keep aligned allocations on mimalloc - #2260
knewstimek wants to merge 2 commits into
Conversation
Signed-off-by: News <knewstimek@users.noreply.github.com>
|
Thank you — a production crash on Windows in code that shipped two days ago is exactly what I want to hear about fast, and a nine-line fix with a clear description is the best possible way to hear it. The principle is right, and it is the one this codebase already tries to hold everywhere else: an allocation and its release must live on the same allocator. Under My working hypothesis for why it crashes — offered as a hypothesis, not a finding, since I cannot reproduce on Windows today: on MinGW the aligned pair can come from two different places. One thing about verification, so you know what I am waiting on and why. Two small questions, neither blocking:
|
|
x64. The original crash was on a larger repository: 2 and 4 workers crashed in |
|
Thank you — that is exactly the detail I needed, and quickly. x64 settles the venue question: our local Windows machine is arm64, so hosted CI's And thank you for being straightforward that the 64-file fixture was added after the fix and has not been shown to fail before it. That is useful to know rather than a problem: it means the fixture is coverage, not yet a regression guard, and it does not hold up this PR. "1 worker completes, 2 and 4 crash in What I will do on our side after this lands is try to turn it into a guard that fails deterministically without the fix — most likely by running the worker teardown against a production-flag Windows build rather than by making the fixture bigger, since size only raises the odds. If that works I will credit you on it. Nothing further is needed from you for this PR. |
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Windows production builds can crash in
_aligned_freewhile cleaning up parallel indexing workers.When
CBM_MEM_GLOBAL_OVERRIDEis enabled, usemi_malloc_alignedandmi_freedirectly. Builds without the override keep using the CRT allocator.Tested with a Windows production build and a 64-file synthetic Go fixture at 2 and 4 workers.