Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow task manager to have children #6771

Merged
merged 14 commits into from
Aug 6, 2020
Merged

Allow task manager to have children #6771

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
894768e
Initial commit
cecton Jul 30, 2020
18c9792
WIP
cecton Jul 30, 2020
3abd7d1
WIP
cecton Jul 30, 2020
c07b416
WIP
cecton Jul 30, 2020
c572f4d
WIP
cecton Jul 30, 2020
b0cf502
WIP
cecton Aug 5, 2020
3d6e03c
WIP
cecton Aug 5, 2020
352a696
changelog
cecton Aug 5, 2020
06e4ecc
Remove Box
cecton Aug 5, 2020
49fb8fb
Make future nicer
bkchr Aug 6, 2020
02c2ece
Revert "Make future nicer"
cecton Aug 6, 2020
4fdf803
Simplify
cecton Aug 6, 2020
932e601
Additional check
cecton Aug 6, 2020
f3b520f
Simplify more
cecton Aug 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Simplify more
  • Loading branch information
@cecton
cecton committed Aug 6, 2020
commit f3b520fca8d10bd5d2b488190125e4d189980aa9
9 changes: 6 additions & 3 deletions client/service/src/task_manager/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,12 @@ impl TaskManager {
Box::pin(async move {
let mut t1 = self.essential_failed_rx.next().fuse();
let mut t2 = self.on_exit.clone().fuse();
let mut t3 = try_join_all(self.children.iter_mut().map(|x| x.future()))
// Never end this future if there is no error
.then(|res| async { Ok(res.map(|_| pending::<()>())?.await) }).boxed().fuse();
let mut t3 = try_join_all(
self.children.iter_mut().map(|x| x.future())
// Never end this future if there is no error because if there is no children,
// it must not stop
.chain(std::iter::once(pending().boxed()))
).fuse();

futures::select! {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this whole function not async? Instead of returning a boxed Future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point!! I think I'm used to do that because you can have async fn in traits.

But it still doesn't work:

error[E0733]: recursion in an `async fn` requires boxing
   --> client/service/src/task_manager/mod.rs:288:40
    |
288 |     pub async fn clean_shutdown(mut self) {
    |                                           ^ recursive `async fn`
    |
    = note: a recursive `async fn` must be

I isolated the suspect. It seems those 2 lines make the function "recursive":

		join_all(children_shutdowns).await;
		completion_future.await;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you remove the internal async move?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes obviously XD

_ = t1 => Err(Error::Other("Essential task failed.".into())),
Expand Down
2 changes: 1 addition & 1 deletion client/service/src/task_manager/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ fn ensure_task_manager_future_continues_when_childs_not_essential_task_fails() {
pin_mut!(t1, t2);

select! {
_ = t1 => panic!("task should not have stopped"),
res = t1 => panic!("task should not have stopped: {:?}", res),
_ = t2 => {},
}
});
Expand Down