rt: blocking runtime doesn't reponse shutdown flag

[bigdogs]

We should check the shutdown flag before the next iteration to interrupt the execution asap, the current implementation doesn't respond to shutdown until queue cleans

Motivation

Solution

[Darksonn]

Hi, thank you for the PR. I missed it when you originally submitted it.

Your PR causes the following test to fail:

tokio/tokio/tests/rt_handle_block_on.rs

Lines 195 to 210 in 7299304

	fn spawn_blocking_started_before_shutdown_continues() {
	use tokio::task::spawn_blocking;
	let rt = rt();
	let _enter = rt.enter();
	let handle = spawn_blocking(|| {
	std::thread::sleep(Duration::from_secs(1));
	42
	});
	rt.shutdown_timeout(Duration::from_secs(1000));
	let answer = Handle::current().block_on(handle).unwrap();
	assert_eq!(answer, 42);
	}

[Darksonn]

According to the test it seems like we (try to) guarantee that tasks spawned before shutdown will complete, but if we really guaranteed that, then #4296 wouldn't be necessary. I don't really understand what's going on with that.

[bigdogs]

According to the test it seems like we (try to) guarantee that tasks spawned before shutdown will complete, but if we really guaranteed that, then #4296 wouldn't be necessary. I don't really understand what's going on with that.

hi, Thanks for the reply.

I'm not sure if tokio is try to guarantee all spawned tasks to complete even runtime is shutdown, but it seems not very conform to the current code design.

Checked the run function, after all tasks in the shared.queue are handled in the first while loop,
we'll check the shutdown flag and then drain all tasks agian to shutdown or run it ( according to the mandotory flag)

            if shared.shutdown {
                // Drain the queue
                while let Some(task) = shared.queue.pop_front() {
                    self.metrics.dec_queue_depth();
                    drop(shared);
                    task.shutdown_or_run_if_mandatory();

So, as my understanding, after the runtime is shutdown, only mandatory tasks will be executed, other tasks should be aborted.

[Darksonn]

Hi, sorry about the delay. It seems like you're correct. We don't guarantee that in general, but it happens to be true for the "first started task". It's not a real guarantee. Please go ahead and delete the test in question.

[Darksonn]

Any updates on this? If you remove the faulty test, then I would be happy to merge this.

[bigdogs]

Any updates on this? If you remove the faulty test, then I would be happy to merge this.

oh, Sorry I've been busy with other projects, I'll find time this weekend to work on this :)

[bigdogs]

there exists other test case failed which I can't figure out why. :(, maybe the test case also consider all spawned task should be executed even if runtime is shutdown. I'll close this pull request as I thinks is just a design choice not a bug :)

    #[test]
    fn pool_shutdown() {
        loom::model(|| {
            let pool = mk_pool(2);

            pool.spawn(track(async move {
                gated2(true).await;
            }));

            pool.spawn(track(async move {
                gated2(false).await;
            }));

            drop(pool);
        });
    }