fix: stop realtime feed before freeing data frames on disconnect (heap corruption, #98)#99
Open
blurxy wants to merge 1 commit into
Open
Conversation
…asCoding#98) TradingSuite.disconnect() cleaned up instrument data contexts (freeing the Polars frames) before stopping the SignalR realtime reader. The reader keeps writing into those frames on native background threads until disconnected, so freeing them first races SignalR threads against Polars memory and corrupts the heap on Windows (STATUS_HEAP_CORRUPTION / 0xc0000374) — an intermittent, unrecoverable abort at disconnect. Reorder disconnect() to call realtime.disconnect() first, quiescing the reader before any data frame is freed. Pure reordering; no behavior change otherwise.
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.
Summary
Fixes a Windows heap-corruption crash (
STATUS_HEAP_CORRUPTION/0xc0000374) that can occur when a multi-instrumentTradingSuitedisconnects while realtime data is flowing. The fix reordersTradingSuite.disconnect()to stop the realtime feed before cleaning up the instrument data contexts.Tracks #98.
Root cause
TradingSuite.disconnect()currently runs cleanup in this order:_disconnect_instrument_contexts()→context.data.cleanup()— frees the Polars DataFrames that hold each timeframe's bars.self.realtime.disconnect()— stops the SignalR background reader.The SignalR reader runs on native background threads and keeps writing incoming quotes/trades into the
RealtimeDataManager's Polars frames until it is stopped. Freeing those frames in step 1 while the reader is still live in step 2 races native SignalR threads against Polars-owned memory. On Windows this manifests asSTATUS_HEAP_CORRUPTION(exit code0xc0000374/3221225477) — an immediate, unrecoverable process abort at disconnect. It is timing-dependent (worst around active-session disconnects / restarts), which is why it presents as an intermittent crash rather than a deterministic one.Fix
Move the
self.realtime.disconnect()call to the top ofdisconnect(), before_disconnect_instrument_contexts()/_disconnect_legacy_single_instrument(). Quiescing the reader first guarantees no native thread is touching a Polars frame while it is being freed. The change is a pure reordering — no behavior change beyond the ordering, no new dependencies.logger.info("Disconnecting TradingSuite...") + # Disconnect realtime FIRST so the SignalR background reader thread is + # quiesced before any Polars data frames it writes into are freed. + if self.realtime: + await self.realtime.disconnect() + if self._instruments: await self._disconnect_instrument_contexts() else: await self._disconnect_legacy_single_instrument() - - # Disconnect realtime - if self.realtime: - await self.realtime.disconnect()Validation
We run the equivalent reordering (quiesce
realtimebefore the per-context cleanups) at the broker layer in a live Windows trading bot on TopstepX/ProjectX with a 4-instrument suite (NQ/ES/MNQ/MES). Before the reorder we saw recurring0xc0000374aborts at disconnect; after it, disconnects are clean across many session restarts and the daily 17:00 ET maintenance rollover. This PR moves that same fix into the SDK so the defaultdisconnect()path is safe for all users.