fix(955): Race condition & TOCTOU remediation -- PR #24 audit fixes #25
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -785,7 +785,19 @@ private void LoadConfig() | |
| selectedTargetCount = activeCount; | ||
|
|
||
| Button modeBtn = GetModeButton(activeMode); | ||
| if (modeBtn != null) | ||
| { | ||
| HighlightModeButton(modeBtn); | ||
| } | ||
| else | ||
| { | ||
| // [Build 954]: Saved mode is deprecated/unrecognized -- normalize both vars to RMA baseline. | ||
| Print("[WARN][954] Unrecognized saved mode '" + activeMode + "' -- falling back to RMA."); | ||
| activeMode = "RMA"; | ||
| selectedConfigMode = "RMA"; | ||
| modeBtn = GetModeButton("RMA"); | ||
| if (modeBtn != null) HighlightModeButton(modeBtn); | ||
| } | ||
|
|
||
| // Apply active mode+count settings to UI | ||
| ApplySettings(fullConfig.GetSettings(activeMode, activeCount)); | ||
|
|
@@ -2986,67 +2998,8 @@ private void SyncAll_Click(object sender, RoutedEventArgs e) | |
|
|
||
| private void ConnectToStrategy() | ||
| { | ||
| // [Build 954]: IPC deprecated. Strategy no longer hosts IPC server (Phase 6 pruning). | ||
| // [Build 955]: Dead code removed. SendCommand() is safely no-oped via tcpStream null guard. | ||
| } | ||
|
Comment on lines
2999
to
3003
There was a problem hiding this comment.
|
||
|
|
||
| private void DisconnectFromStrategy() | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -369,8 +369,20 @@ private bool HandleOrderCancelled(Order order) | |||||||||||||||||||
| { | ||||||||||||||||||||
| if (kvp.Value.OldOrder == order && activePositions.TryGetValue(kvp.Key, out var pos)) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| // Build 955: Snapshot qty under stateLock -- single atomic read for both check and use. | ||||||||||||||||||||
| int _stopQty; | ||||||||||||||||||||
| lock (stateLock) { _stopQty = pos.RemainingContracts; } | ||||||||||||||||||||
| if (_stopQty > 0) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| CreateNewStopOrder(kvp.Key, _stopQty, kvp.Value.StopPrice, kvp.Value.Direction); | ||||||||||||||||||||
| // Build 950: Restore OCO-cascade-cancelled targets after stop replacement. | ||||||||||||||||||||
| if (kvp.Value.BracketRestorationNeeded && kvp.Value.CapturedTargets != null) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| TargetSnapshot[] _mSnap = kvp.Value.CapturedTargets; | ||||||||||||||||||||
| string _mKey = kvp.Key; | ||||||||||||||||||||
| TriggerCustomEvent(o => RestoreCascadedTargets(_mKey, _mSnap), null); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (pendingStopReplacements.TryRemove(kvp.Key, out _)) Interlocked.Decrement(ref pendingReplacementCount); | ||||||||||||||||||||
| handled = true; | ||||||||||||||||||||
| break; | ||||||||||||||||||||
|
|
@@ -578,6 +590,39 @@ private void HandleMatchedFollowerOrder(string matchedEntry, PositionInfo matche | |||||||||||||||||||
| } | ||||||||||||||||||||
| else | ||||||||||||||||||||
| { | ||||||||||||||||||||
| // Build 950: Follower stop replacement -- mirrors HandleOrderCancelled master path. | ||||||||||||||||||||
| // Follower stop cancels arrive via OnAccountOrderUpdate (not OnOrderUpdate), so | ||||||||||||||||||||
| // HandleOrderCancelled never fires for them. Match pendingStopReplacements here. | ||||||||||||||||||||
| // This block is in the else branch because stop orders are not in entryOrders. | ||||||||||||||||||||
| if (order.Name.StartsWith("Stop_") || order.Name.StartsWith("S_")) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| foreach (var _psr in pendingStopReplacements.ToArray()) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| if (_psr.Value.OldOrder == order) | ||||||||||||||||||||
|
There was a problem hiding this comment. The follower stop-cancel match uses reference equality only (
Suggested change
|
||||||||||||||||||||
| { | ||||||||||||||||||||
| PositionInfo _rPos; | ||||||||||||||||||||
| // Build 955: Move guard inside lock -- check and use same atomic snapshot. | ||||||||||||||||||||
| if (activePositions.TryGetValue(_psr.Key, out _rPos)) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| int _rQty; | ||||||||||||||||||||
| lock (stateLock) { _rQty = _rPos.RemainingContracts; } | ||||||||||||||||||||
| if (_rQty > 0) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| CreateNewStopOrder(_psr.Key, _rQty, _psr.Value.StopPrice, _psr.Value.Direction); | ||||||||||||||||||||
| if (_psr.Value.BracketRestorationNeeded && _psr.Value.CapturedTargets != null) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| TargetSnapshot[] _snap = _psr.Value.CapturedTargets; | ||||||||||||||||||||
| string _rKey = _psr.Key; | ||||||||||||||||||||
| TriggerCustomEvent(o => RestoreCascadedTargets(_rKey, _snap), null); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } // if (_rQty > 0) | ||||||||||||||||||||
| } // if (activePositions.TryGetValue) | ||||||||||||||||||||
| if (pendingStopReplacements.TryRemove(_psr.Key, out _)) | ||||||||||||||||||||
| Interlocked.Decrement(ref pendingReplacementCount); | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+601
to
+624
There was a problem hiding this comment.
In the master path ( If a position is cleaned up from Adding a diagnostic log when the position is missing improves observability: if (activePositions.TryGetValue(_psr.Key, out _rPos))
{
// ... qty check and CreateNewStopOrder ...
}
else
{
Print(string.Format("[B955] WARN: Follower stop cancel matched pending '{0}' but position not in activePositions -- pending cleaned up without replacement.", _psr.Key));
}
if (pendingStopReplacements.TryRemove(_psr.Key, out _))
Interlocked.Decrement(ref pendingReplacementCount);
return; |
||||||||||||||||||||
| } | ||||||||||||||||||||
| Print(string.Format("[SIMA] Follower order terminal: {0} on {1} ({2}) | Id={3}", order.Name, acctName, reason, order.OrderId)); | ||||||||||||||||||||
| RemoveGhostOrderRef(order, reason); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section claims
BUILD_TAG = "950"in the listed file changes, but the strategyBUILD_TAGwas updated to 955 in this PR. Please update the walkthrough to reflect the correct build tag (or make the note build-agnostic) to avoid future confusion during verification/audits.