Immediately drop new task when runtime is shut down #3752
Conversation
Darksonn
added
A-tokio
|
Looks related to: #3708 |
| } | ||
| } | ||
| } | ||
| drop(remote_queue); |
There was a problem hiding this comment.
If the drop here is significant (which it probably is because this is a lock guard), it probably is worth adding a comment saying so to avoid removing it in the future by accident.
There was a problem hiding this comment.
Does the drop here in interact with the assert below?
There was a problem hiding this comment.
No, I only put it in to make it more visually clear where the lock is dropped.
| @@ -429,7 +436,9 @@ impl Schedule for Arc<Shared> { | |||
| // safety: the task is inserted in the list in `bind`. | |||
| unsafe { cx.tasks.borrow_mut().owned.remove(ptr) } | |||
| } else { | |||
| self.queue.lock().push_back(Entry::Release(ptr)); | |||
| if let Some(queue) = self.queue.lock().as_mut() { | |||
There was a problem hiding this comment.
So what happens if this is None? Will the task leak?
There was a problem hiding this comment.
After digging in, this looks fine, a comment explaining why would be nice.
| let jh1 = handle.spawn(futures::future::pending::<()>()); | ||
|
|
||
| drop(rt); | ||
|
|
||
| let jh2 = handle.spawn(futures::future::pending::<()>()); | ||
|
|
||
| let err1 = jh1.now_or_never().unwrap().unwrap_err(); | ||
| let err2 = jh2.now_or_never().unwrap().unwrap_err(); |
There was a problem hiding this comment.
@carllerche The unwrap() on err1 here sometimes fails with the threaded scheduler. Isn't the drop call supposed to wait until the runtime is fully shut down?
There was a problem hiding this comment.
This is fixed by the "Fix loom test" commit
Attempt to resolve #3548 by having each runtime's spawn method drop the new task.
I was only able to get it to work for the basic scheduler.