Release completed cancellable tasks#997
Conversation
|
I've assigned @tankyleo as a reviewer! |
Avoid retaining completed Tokio task allocations for the node lifetime while preserving shutdown cancellation and restart semantics. Co-Authored-By: HAL 9000
Remove pending LSPS request state when callers time out or are cancelled so unresponsive services cannot grow request maps. Co-Authored-By: HAL 9000
Clear per-peer connection state when the leading task is cancelled so later callers can retry and existing subscribers do not hang. Co-Authored-By: HAL 9000
Restore wallet sync status and notify waiting callers when the task performing a sync is cancelled, allowing later sync attempts to run. Co-Authored-By: HAL 9000
3553a89 to
fb5fbc8
Compare
|
Rebased after #956 landed. |
elnosh
left a comment
elnosh
left a comment
There was a problem hiding this comment.
ACK fix in 08d3b5d
I'm less familiar with the code in some of the other commits, specially in liquidity. Some of the changes implementing the different Guards took me quite a bit to understand so I would've appreciated some comments but they look correct. Specifically:
- reason for
PendingRequestGuardneeding the tokens for the pending requests inliquidity/mod.rs PendingRequestinliquidityseparating the ownerSenderfrom the otherfollowersVec<oneshot::Sender<T>>. Inconnection.rsthere's just one vec for all the senders.
These make sense now but not particularly obvious so adding comments is just a suggestion, I'm fine with it as-is
| debug_assert!( | ||
| false, | ||
| "Failed to send connection result to subscribers: {:?}", | ||
| e | ||
| ); |
There was a problem hiding this comment.
similar to fb5fbc8 this debug_assert could be removed as receiver from this pending connection attempt might have been dropped.
3 participants
Avoid retaining completed Tokio task allocations for the node lifetime while preserving shutdown cancellation and restart semantics. This fixes a considerable memory leak as previously we'd retain the
JoinSeted task's allocations untilstop/join_next, as tokio thankfully only notes in the https://docs.rs/tokio-util/latest/tokio_util/task/task_tracker/struct.TaskTracker.html#comparison-to-joinset.Thanks to @elnosh for reporting this leak.