runtime_service: don't panic if to_background_tx fails to upgrade after chain removal#2248
runtime_service: don't panic if to_background_tx fails to upgrade after chain removal#2248SAY-5 wants to merge 2 commits into
Conversation
…er chain removal Signed-off-by: SAY-5 <say.apm35@gmail.com>
There was a problem hiding this comment.
I'm not familiarised enough with the codebase, but I'm wondering... the .unwrap() has been there for quite a long time, and it hasn’t caused issues until 3.1.1.
To me, it looks like it was written based on an assumption that may no longer hold true, and that could matter. Simply turning it into a noop when background_tx.upgrade() returns None might have other implications as well.
My question is, what’s actually causing this channel to close instead of cleanly shutting down whatever process it was handling?
|
The fix has been adopted in: Would suggest creating PRs against the https://github.com/paritytech/smoldot, we plan to keep this as an archive / history repo 🙏 @SAY-5 thanks for contributing! |
|
Thanks for the pointer, closing this in favour of the smoldot fix. |
Fixes paritytech/smoldot#3238 (the same code path lives here at
light-base/src/runtime_service.rs:1689).When a chain object is removed concurrently with an in-flight
SubscribeAllresponse,background.to_background_tx.upgrade()returnsNoneand theunwrap()panics the runtime task, this is the panic users started hitting after upgrading to 3.1.1 in browser dapps. The receiver is going away anyway in that case; skip the response withcontinue.