Fix Windows indexing recovery and source settings

[knewstimek]

Windows indexing could crash when C++ aligned delete sent a mimalloc pointer to UCRT _aligned_free. Failed-worker recovery could also exhaust _wmktemp names after 26 retained logs and replace the original worker error with a spawn error.

This routes aligned delete through the matching allocator, creates worker artifacts exclusively with random names, and keeps the first failure if recovery setup fails. Source reads now accept UTF-8 BOM and CP949/EUC-KR. File-specific defines and include paths from compile_commands.json, including MSVC flags, reach extraction; changing those inputs triggers reindexing.

Encoding scope: other legacy encodings such as GBK, GB18030, Big5, and Shift-JIS are not supported here. Automatically treating every non-UTF-8 file as CP949 can misread them; broader support needs an explicit encoding choice.

To reproduce the temp-file failure on Windows, retain more than 26 files from a XXXXXX template. The new platform test retains 64. A parallel C++ indexing case that previously faulted in _aligned_free now completes.

Checks: Windows production build, binary composition check, and platform/pipeline tests (309 passed, 4 skipped).

[github-actions]

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. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

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.

[DeusData]

Thank you — and first, some news: #2260 merged tonight as b82cf7b, with pr-smoke on windows-latest green on the final head. It ships in the next patch release.

This PR contains the better half of that story. In #2260's thread I could only hypothesise why _aligned_free crashed; you have now found the actual mechanism: LLVM-MinGW's static C++ runtime reaches _aligned_malloc through the linker wrapper, but its aligned operator delete can call the CRT import pointer __imp__aligned_free directly, and --wrap=_aligned_free never rewrites an import-pointer reference. So a mimalloc-owned block still ends up in UCRT's _aligned_free. Providing the six C++ ABI aligned-delete entry points so every aligned new/delete pair reaches the same owner-aware deallocator is the right shape of fix, and the comment explaining it is exactly the kind worth having.

That is also why I am asking you to split this PR — so the crash fixes can land fast instead of waiting for the features bundled with them. This one commit makes five independent changes:

1. Aligned delete → matching allocator (mem_override_win.c). A production crash fix. Wants its own PR, today.

2. Worker temp files and recovery errors: _wmktemp exhausting after 26 retained logs, random exclusive names instead (compat.c, the platform test retaining 64), the artifact_create_failed log line in index_supervisor.c, and — in mcp.c — no longer letting a recovery setup failure overwrite the worker failure that started recovery. A second real bug fix: replacing the original worker error with a spawn error is precisely the failure mode that makes crashes undiagnosable. Its own PR.

3. cbm_setenv on Windows: _putenv_s → _wputenv_s (compat.h). This one is not in the PR description at all. The reasoning in your new comment is plausible (_putenv_s rejecting UTF-8 the ANSI code page cannot represent), but the comment it replaces documents a deliberate choice — keep the CRT's narrow environment useful for legacy getenv callers, then repair the process environment with SetEnvironmentVariableW. Changing which CRT table gets written deserves its own small PR, a sentence on what a narrow getenv of that variable returns afterwards, and a test that fails before it.

4. Source encoding: UTF-8 BOM, plus CP949/EUC-KR decoding (source_encoding.c/.h, CBM_READ_ENCODING, and the five passes that now read through it). Accepting a BOM is an uncontroversial fix and can stand alone. Legacy-encoding decoding is a feature with two open questions:

   • Direction. You name the hard part yourself: treating every non-UTF-8 file as CP949 misreads GBK, GB18030, Big5 and Shift-JIS. Most likely the answer is an explicit, configured encoding rather than a guessed one. There is also a behaviour change for everyone else: with the new CBM_READ_ENCODING status, a file that is neither valid UTF-8 nor valid CP949 is now skipped ("unsupported source encoding"), where today it is indexed — think of one Latin-1 byte in a comment of an otherwise ASCII file. And a Latin-1 file whose bytes happen to form valid CP949 pairs would be silently rewritten into Korean text. Both need to be decided, not inherited.
   • Dependency. The non-Windows path calls iconv, and nothing links it: that is the pr-smoke (macos-14) red ("_iconv_open", referenced from: _cbm_source_transcode_utf8 … ld: symbol(s) not found for architecture arm64). Adding -liconv would fix macOS, but our Linux release binary is statically linked, and to my knowledge glibc's iconv loads its converters at run time, which a static binary generally cannot do — so the feature could link everywhere and still silently not work in the binary most people run. A new system-library dependency is a project-level decision in any case.

   Both belong in an issue first; the contribution rules ask for that discussion before the code.

5. compile_commands.json per-file defines and include paths, including MSVC flags, reaching extraction, and re-indexing when they change (pass_compile_commands.c +194, pipeline_incremental.c +50). Valuable and plausible, but it changes what gets extracted and when a re-index triggers — by our rules that is a design discussion in an issue first, and it deserves its own before/after numbers on a real MSVC project.

With #1 and #2 as separate small PRs I can review each the day it arrives; bundled, the whole thing waits for the slowest part.

Three mechanical things that apply to every one of them:

• DCO sign-off. The commit has no Signed-off-by: trailer, which is why dco is red. git commit -s on each new commit (a noreply identity is fine, as long as it matches the commit author). Force-pushing your own branches is completely fine here.
• Formatting. lint / lint is red on clang-format only (mem_override_win.c, compat.c, source_encoding.c). make -f Makefile.cbm lint-ci reproduces it; please use an LLVM-distributed clang-format — some standalone builds report whole-file drift that is not real. Because the test matrix only starts once lint passes, nothing past lint has run on this PR yet.
• A test that fails without the fix, per change. #2 already has one — platform_mkstemp_retained_files_exceed_crt_namespace is exactly right. For #1 the honest difficulty is the one we met on #2260: CBM_MEM_GLOBAL_OVERRIDE is production-flags-only, so the unit-test legs never compile that branch and only pr-smoke (windows-latest) exercises it. If your parallel C++ indexing case reliably faulted before the change, say so in the PR and describe the repository shape that triggers it — a deterministic guard for this class (worker teardown plus aligned C++ allocation against a production-flag Windows build) is something I owe you from #2260, and your repro is the best starting point for it.

One detail for #1 while you are in there: the hunk declares __real__aligned_free and routes CRT-owned aligned blocks to it rather than to plain free — correct, because a CRT aligned block carries its own bookkeeping header. Please keep that comment; it is the sort of thing someone "simplifies" two years later.

Thank you for digging past the first fix to the real cause. That is rare, and it is appreciated.

[knewstimek]

Split the crash fixes as requested: #2281 covers aligned delete, and #2282 covers worker temp files and recovery errors. I will take legacy encoding and compile_commands changes to issues for design discussion. Closing this bundled PR.

[knewstimek]

The cbm_setenv change is now in #2290. Its test covers a UTF-8 value that fails under a legacy ANSI code page and documents what narrow getenv returns after the wide CRT update.

I checked the BOM hunk before splitting it out. The vendored Tree-sitter lexer already skips a BOM at byte zero, and pipeline_bom_stripping already indexes BOM-prefixed Go source. A BOM-prefixed C #define also parsed without an error in a focused grammar check. Removing those three bytes in the source readers would shift AST byte offsets relative to the file on disk, so I have left that hunk out. If there is a BOM-prefixed source case that still fails, I can use it for a focused regression PR.