fix(history): per-request events to avoid KeyError + shared-event race#52
Merged
rundef merged 1 commit intorundef:mainfrom Apr 20, 2026
Merged
Conversation
The historical bar/tick retrieval had two race conditions stemming from a
single shared asyncio.Event per plant:
A) Empty response: when Rithmic returns only the is_last_bar marker (no
data bars), the event fires within 5s so the TimeoutError fallback
is skipped, and historical_*_data.pop(key) raises KeyError because
no callback ever populated that key.
B) Concurrent calls: the second get_historical_time_bars call replaces
the shared event instance. The first call's response then sets the
second call's event, waking caller B before its data has arrived.
Replace the shared event with a dict of per-request events keyed by
"{symbol}_{type}" (time bars) or "{symbol}" (ticks). Each call allocates
its own Event, registers it, waits, and cleans up in a finally. The pop
uses a default of [] so empty responses return an empty list instead of
raising.
In _process_response, the is_last_bar path now looks up the specific
event by key from response.symbol + response.type and sets only that one.
3 regression tests added under tests/test_history_races.py. Full suite:
29 passed (26 existing + 3 new).
Symptom that triggered this fix: KeyError: 'MYMM6_2' on live_propfirms
during low-volume backfill (seen 2026-04-13 03:29 and 05:35 ET).
Owner
|
Thank you for your contribution ! |
Contributor
Author
|
I typically run it live for a few days before pushing the PR |
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
2 participants
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
Two race conditions in
HistoryPlantstemming from a single sharedasyncio.Eventper plant are fixed by switching to per-request events keyed by{symbol}_{type}(time bars) or{symbol}(ticks).Bug A — KeyError on empty response
When Rithmic returns only the
is_last_barmarker with no data bars, the shared event fires within the 5s timeout, so theTimeoutErrorfallback path is skipped.historical_*_data.pop(key)then raisesKeyErrorbecause no callback ever populated that key.Fix:
pop(key, [])with an empty-list default — empty responses return[]instead of crashing.Bug B — Concurrent callers share one event
The second
get_historical_time_barscall reassignsself.historical_time_bar_eventto a fresh Event, overwriting the first caller's event. The first response then fires the second caller's event, waking caller B before its own data has arrived.Fix: Each call allocates its own
asyncio.Event, registers it in a dict keyed by{symbol}_{type}(or{symbol}for ticks), awaits, and cleans up in afinally. In_process_response, theis_last_barpath looks up the specific event by key fromresponse.symbol+response.typeand sets only that one.Real-world trigger
KeyError: 'MYMM6_2'on production bots during low-volume historical-bar backfill (seen 2026-04-13 03:29 and 05:35 ET).Test plan
tests/test_history_races.py:test_empty_response_returns_empty_list— covers Bug A for time barstest_empty_tick_response_returns_empty_list— covers Bug A for tickstest_concurrent_different_symbols— covers Bug B with two callers racingKeyError: 'MYMM*_2'logged sinceFiles changed
async_rithmic/plants/history.py— per-request events + pop-with-defaulttests/test_history_races.py— 3 new regression tests