Skip to content

Harden Windows crash handling, IPC lifetimes, and release symbols - #15

Merged
middaysan merged 33 commits into
mainfrom
fix/windows-crash-memory-safety
Aug 31, 2026
Merged

middaysan merged 33 commits into
mainfrom
fix/windows-crash-memory-safety

Conversation

@middaysan

@middaysan middaysan commented Aug 20, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • remove the global mimalloc override from both the Windows application and Linux beta binary, allowing each binary to use Rust's platform default allocator
  • keep Windows named-pipe OVERLAPPED state, event handles, and caller-owned I/O buffers alive until Windows reports a terminal completion result
  • treat both WAIT_TIMEOUT and ERROR_IO_INCOMPLETE as pending overlapped completion states
  • stop and drop the saved-rule forwarding server before releasing the primary-process guard
  • build future stable Windows releases with line-table debug information, verify the EXE/PDB identity, and publish the matching cpu_affinity_tool.pdb
  • save bounded local Windows crash reports for main-thread Rust panics and native UI-loop errors, then surface the newest validated crash context in Activity after the next startup
  • strengthen allocator, workflow, symbol-verifier, and lifecycle contract coverage and update the release documentation

Relationship to #14 and causal limits

Issue #14 currently contains reports from two distinct GitHub accounts: the original report describes the application closing after it was minimized and a game was launched, while a later report describes a reproducible exit when closing Black Desert Online after the application changed its affinity. These may or may not share a failure mechanism.

The later Windows Application Error event reports exception 0xc0000005, module timestamp 0x6A581616, and fault RVA 0xA8B6F6. The timestamp matches the official v1.5.0 release EXE, whose SHA-256 is:

97a24947b961b59de2436ed1f75af182ba6320fc88f3c65bbe5840269386c271

The reporter did not provide their executable hash, so the matching timestamp is build-consistency evidence rather than cryptographic proof of identical files.

Binary and source mapping of the official v1.5.0 EXE places RVA 0xA8B6F6 at _mi_page_malloc_zero+0x26 in the mimalloc allocation fast path, at a read through the page free-list pointer. This establishes where the access violation surfaced, not where or by whom the invalid state was created.

Release Rust wrapper FFI crate Bundled mimalloc engine
v1.4.0 mimalloc 0.1.48 libmimalloc-sys 0.1.44 2.2.4
v1.5.0 mimalloc 0.1.52 libmimalloc-sys 0.1.49 3.3.2

Version 1.5.0 upgraded an allocator already present in v1.4.0; it did not introduce mimalloc for the first time. Removing the custom allocator is an isolation and mitigation change, not a claim that an internal mimalloc defect is the root cause. It may also mask an earlier invalid write, use-after-free, double-free, or another bug whose manifestation depends on heap layout. No anti-cheat or specific upstream mimalloc issue is treated as proven causal evidence.

Independently found Windows IPC defects

The audit found a separate lifetime defect in the saved-rule named-pipe transport. After CancelIoEx, the old timeout path could return while Windows still owned an asynchronous ReadFile or WriteFile. It preserved only the OVERLAPPED and event, while the caller-owned buffer could be dropped.

The updated path:

  • keeps polling while GetOverlappedResultEx reports WAIT_TIMEOUT or ERROR_IO_INCOMPLETE
  • does not return until Windows reports terminal completion
  • applies the same handling when the initiating overlapped API call returned success
  • explicitly stops and drops the forwarding server before releasing the primary guard

The affected IPC code existed in both v1.4.0 and v1.5.0. There is no evidence that saved-rule IPC was active in either reported game scenario, so this is independent hardening rather than a claimed explanation for #14.

Local crash reports and user logs

On Windows, a main/UI-thread Rust panic or an eframe::run_native error writes a bounded, UTF-8 local report before the process exits. The report is stored under the active data directory, never uploaded automatically, and remains the complete artifact a user can review and attach to an issue.

The next startup indexes reports on a background thread. Once that validated scan completes, Activity keeps the newest report's type, timestamp, reason, and full-report path as a dedicated crash-context entry. Activity Clear removes ordinary activity but leaves this latest crash context visible until a refresh replaces it or the report is deleted deliberately from Crash reports.

This is diagnostics support, not a claim that every native crash is captured: forced termination, native access violations, aborts, stack overflow, OOM, power loss, anti-cheat termination, and background task panics can still leave no report.

Windows symbol and release contract

Windows CI and the stable release workflow now use the same line-tables-only release helper. Before publication, the verifier requires:

  • the supplied PDB basename to match the EXE CodeView record
  • the expected cpu_affinity_tool.pdb artifact path
  • the EXE and PDB to have the same non-empty CodeView GUID
  • the EXE and PDB to have the same age

CI exercises positive plus missing, empty, wrong-basename, and mismatched-identity cases. The stable workflow fails before upload if either required file is absent, and GitHub Release publication remains fail-closed for missing declared artifacts.

This does not retroactively create a matching PDB for the already published v1.5.0 EXE: rebuilding produces a different CodeView identity. The PDB is not loaded at runtime, but it is a new public release asset and can contain diagnostic symbol, source-path, and line metadata.

The shared Cargo release profile and Linux beta artifact set/debug-information policy remain unchanged. The allocator change itself affects both Windows and Linux binaries.

Compatibility and risk

  • no public API, persisted-state schema, UI, affinity, priority, process-launch, or monitoring behavior change is intended
  • allocation latency, throughput, fragmentation, and RSS can change on both Windows and Linux; these effects have not been benchmarked
  • IPC error mapping is intended to remain unchanged, but cleanup can continue past the requested timeout until Windows confirms terminal I/O completion
  • shutdown retains the primary guard until the forwarding server thread has stopped
  • future stable releases add a public PDB and may have different EXE/PDB sizes and binary identities

Verification

Local checks completed on 3eae2446e26600b305a8a8b41750bac709e9c049:

  • git diff --check
  • cargo fmt --all -- --check
  • cargo test --locked --manifest-path libs/os_api/Cargo.toml — 47 passed
  • cargo test --locked --features windows --bin cpu-affinity-tool — 280 passed
  • cargo test --locked --features linux --bin cpu-affinity-tool-linux — 269 passed
  • Windows and Linux cargo clippy --locked ... -D warnings
  • 25 repeated client-timeout plus 25 silent-client shutdown IPC runs
  • scripts/build-windows-release.ps1 — [optimized + debuginfo]
  • scripts/test-windows-pdb-verifier.ps1 — positive and four negative cases passed
  • scripts/assert-windows-release-manifest.ps1 — requireAdministrator, uiAccess=false
  • scripts/test-windows-crash-reports.ps1 — passed
  • Windows and Linux release builds — passed

GitHub Actions for 3eae2446e26600b305a8a8b41750bac709e9c049:

Manual validation still required before leaving draft

  • repeat the reported launch/minimize/close scenarios on an affected Windows system, recording the exact executable hash and scenario
  • exercise saved-rule forwarding with the GUI already running and from a cold start
  • stop the application while an IPC client is connected but silent
  • force the supported panic/native-loop probe path and confirm that the next normal startup shows the retained crash context in Activity, including after Clear
  • confirm the stable release workflow publishes both cpu-affinity-tool.exe and matching cpu_affinity_tool.pdb
  • collect a matching full dump and controlled allocator-only versus IPC-only A/B results before making any root-cause claim

Refs #14

@middaysan middaysan changed the title Fix allocator crash and overlapped I/O lifetime Use the platform default allocator and harden Windows IPC lifetimes Aug 26, 2026
@middaysan middaysan changed the title Use the platform default allocator and harden Windows IPC lifetimes Harden Windows crash handling, IPC lifetimes, and release symbols Aug 28, 2026
@middaysan
middaysan marked this pull request as ready for review August 31, 2026 20:15
@middaysan
middaysan merged commit 0873040 into main Aug 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant