Conversation
bkchr
added
A0-please_review
| join_all(children_shutdowns).await; | ||
| completion_future.await; | ||
| drop(keep_alive); | ||
| keep_alive |
There was a problem hiding this comment.
can you explain this change i.e, is drop not required because it's consumed by the future?
niklasad1
left a comment
niklasad1
left a comment
There was a problem hiding this comment.
LGTM expect that I would like an explanation for the removed drop in clean_shutdown for explicit approval
Yeah, I'm about to add some comment to the code ;) |
|
|
||
| // The keep_alive stuff is holding references to some RPC handles etc. These | ||
| // RPC handles spawn their own tokio stuff and that doesn't like to be closed in an | ||
| // async context. So, we move the deletion to some other thread. |
There was a problem hiding this comment.
how did it work previously? or is this a change in behavior from tokio 0.2 to 1.0?
There was a problem hiding this comment.
I think tokio 1.0 can only detect tokio 1.0. So it did not detected the shutdown being async before. And as both used 0.1 or whatever, tokio itself provided an async function.
This is entirely some hackery stuff in here. When we refactor all of this, there should not be any "keep_alive", because this itself is just a hack...
|
bot merge force |
|
Trying merge. |
ghost
merged commit 
* Upgrade tokio to 1.10 * Fix test runner * Try fix it * Update Cargo.lock * Review feedback * ahhhh * FML * FMT * Fix tests
3 participants
polkadot companion: paritytech/polkadot#3695