Skip to content

Order requests are still sent to the brokerage minutes after the algorithm stopped on a runtime error #9771

Description

@AlexCatarino

Expected Behavior

When a live algorithm stops on a runtime error, order requests that are still queued should not be sent to the brokerage. The platform has already flipped the deploy to not-running and result reporting has ended, so anything submitted past that point is invisible to the user and carries stale intent.

Actual Behavior

OrderRequestProcessingPool.Dispose() drains the queue instead of discarding it, and the shutdown cancellation cannot interrupt a request already inside a brokerage call. Every queued order request is still delivered, and with a slow brokerage the delivery can land many minutes after the stop.

Observed on a live deploy against a real broker account: four market orders reached the broker with real broker ids 9 to 10.5 minutes after the algorithm had stopped.

19:14:59.80  ERROR:: AlgorithmManager.Run(): Stopping, encountered a runtime error
19:15:00.12  ERROR:: Cloud.Api.Update(): The algorithm status is not running, it can not be updated
             ... the deploy is already considered dead ...
19:23:48.45  OrderID: 3 PCG  Status: Submitted  Quantity:  278
19:25:35.42  OrderID: 4 INTC Status: Submitted  Quantity:  -13
19:25:36.41  OrderID: 5 GPRE Status: Submitted  Quantity:  -82
19:25:37.41  OrderID: 6 BBAI Status: Submitted  Quantity: 8436

Two aggravating details:

  • LiveTradingResultHandler had already ended, so no terminal order event was recorded. The orders exist at the broker and nowhere in the user's live results.
  • They are market orders priced on ~10 minute stale intent.

Mechanism

Three things combine, in Engine/TransactionHandlers/OrderRequestProcessingPool.cs Dispose():

  1. Shutdown drains rather than cancels. _readyQueue.CompleteAdding() lets the workers finish everything queued and parked. The comment above the loop states this is deliberate. It is defensible for a clean stop, where queued liquidations should land; it is not what you want when the stop is a runtime error.
  2. Cancellation cannot interrupt an in flight request. StopSafely (Common/Extensions.cs:605) cancels the shared CTS, but no token is threaded into _processRequest -> BrokerageTransactionHandler -> PlaceOrder. BusyBlockingCollection.GetConsumingEnumerable only observes the token back at TryTake, i.e. after the order has already gone out. The 60s timeouts therefore do not stop anything.
  3. The budget is per thread and serial. 60s Join + 60s StopSafely, one thread at a time, with no overall ceiling. Five workers is 5 x 120s, which is the ~10 minute window above.

Reproducing the problem

Branch with two tests: https://github.com/AlexCatarino/Lean/tree/repro-orders-submitted-after-shutdown

Tests/Engine/BrokerageTransactionHandlerTests/OrderRequestProcessingPoolShutdownTests.cs, both passing against current master, no broker outage needed:

  • QueuedRequestsAreStillSentToTheBrokerageAfterShutdownStarts (288 ms) - one worker parked in a stub brokerage call, four orders queued behind it, then Dispose(). All four are still delivered, in arrival order, every one of them recorded as submitted after the shutdown began.
  • ShutdownDoesNotInterruptAnInFlightBrokerageCall ([Explicit], 2 m 30 s) - a request that ignores cancellation for 150s. Dispose() measurably spends its full 120s budget without interrupting anything, and the order is submitted after the pool has been fully disposed.

Note these tests assert the current behaviour, so they are a repro rather than a regression guard and will need inverting alongside a fix.

System Information

Reproduced on current master (f24fc0d3d), .NET 10, Windows. The live deploy was on the QuantConnect cloud against Tradier, but the defect is brokerage agnostic: any brokerage whose calls block longer than the shutdown budget reproduces it.

Possible fix direction

Needs an owner's call on policy before anything is written:

  1. Discard, don't drain, when the stop is an error. Distinguish error path teardown from a clean stop, purge the pending queue in the former and mark each request Canceled/Invalid so the user sees them. Worth confirming this is safe for the liquidation case.
  2. Thread a CancellationToken into the order processing path so a cancelled pool actually interrupts, instead of the token being observable only between requests. This is what makes the 60s timeouts mean something.
  3. Bound total shutdown, not per thread. A single overall deadline with the remaining threads cancelled together caps the exposure, instead of 120s x threadCount serial with no ceiling.
  4. Whatever is submitted must be reported. If a drain is kept, result reporting has to outlive it, otherwise these orders stay invisible.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions