Skip to content

fix: clean up orderBookResolvers after resolution in probable websocket#573

Merged
realfishsam merged 1 commit into
mainfrom
fix/550-probable-resolver-leak
May 24, 2026
Merged

fix: clean up orderBookResolvers after resolution in probable websocket#573
realfishsam merged 1 commit into
mainfrom
fix/550-probable-resolver-leak

Conversation

@realfishsam
Copy link
Copy Markdown
Contributor

Fixes #550

@realfishsam
Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Adds a .finally() handler to the watchOrderBook promise in the Probable WebSocket that removes the resolver from the queue after the promise settles (whether by data arrival or timeout). This prevents unbounded growth of the orderBookResolvers map when timeouts occur.

Blast Radius

  • Core: core/src/exchanges/probable/websocket.ts only
  • No OpenAPI, SDK, or type changes

Findings

  1. The resolver variable is declared with let (uninitialized) and then assigned inside the Promise constructor. The resolver! non-null assertion on line where indexOf is called is technically safe because the .finally() can only fire after the Promise constructor has run, but it relies on a subtle execution order guarantee. A cleaner pattern would be to initialize resolver inline or use a sentinel, though this is non-blocking.
  2. Potential double-removal is harmless: resolveOrderBook() (line 112-118) already clears the entire queue via this.orderBookResolvers.set(tokenId, []) on data arrival. The .finally() then calls indexOf on the fresh empty array and finds nothing -- no bug, just a no-op.
  3. splice mutates the array in-place -- this is acceptable here because the queue array is an internal mutable structure, not user-facing data.
  4. The close() method (line 131) already rejects all pending resolvers and clears the map, so .finally() cleanup interacts safely with shutdown.

PMXT Pipeline Check

  • Field propagation: N/A
  • OpenAPI sync: N/A
  • Type safety: OK (the resolver! assertion is safe given execution semantics)

Semver Impact

patch -- internal memory leak fix, no API or behavioral change

Risk

Not verified with a live Probable WebSocket connection. The interaction between resolveOrderBook (replaces entire array) and the .finally() (splices individual entry) is benign but worth noting in case the resolution strategy changes later.

@realfishsam realfishsam merged commit 4d996a2 into main May 24, 2026
11 of 12 checks passed
@realfishsam realfishsam deleted the fix/550-probable-resolver-leak branch May 24, 2026 17:04
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.

Unbounded: core/src/exchanges/probable/websocket.ts — orderBookResolvers leak on watchTimeout

1 participant