Move inject queue to `tokio::runtime::task` #3939

Darksonn · 2021-07-07T14:26:27Z

This PR moves the inject queue into the tokio::runtime::task module. This is an incremental change towards the goal of isolating the unsafety in the task module and providing a safe API to tokio::runtime::task.

The main two changes in this PR are:

Move the inject queue from src/runtime/queue.rs to src/runtime/task/inject.rs.
Change Inject::push_batch to an iterator-based interface.

The work-stealing local run queue will continue to have unsafe code despite being outside the task module, but its unsafety is going to be unrelated to the invariants of the tokio::task module.

It may be easier to read this change one commit at the time. The first commit performs no changes to the code besides moving it around and running rustfmt.

carllerche

Change looks great to me, no comments. I asked @hawkw to take a look though as well.

hawkw

looks good to me --- i commented on some very minor nits, but no blockers!

hawkw · 2021-07-09T21:07:35Z

tokio/src/runtime/queue.rs

+fn make_fixed_size<T>(buffer: Box<[T]>) -> Box<[T; LOCAL_QUEUE_CAPACITY]> {
+    assert_eq!(buffer.len(), LOCAL_QUEUE_CAPACITY);
+
+    // SAFETY: We check that the length is correct.
+    unsafe { Box::from_raw(Box::into_raw(buffer).cast()) }
+}


is this really necessary? is there a reason we can't just replace the Vec::new with a boxed array literal? if we can't do that, it would be nice to have a comment explaining why, IMO.

This is the alternative:

Box::new([ UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), UnsafeCell::new(MaybeUninit::new()), ])

ah, um. on second thought let's maybe not do that.

tokio/src/runtime/queue.rs

hawkw · 2021-07-09T21:16:27Z

tokio/src/runtime/queue.rs

-
-    /// Linked-list tail
-    tail: Option<NonNull<task::Header>>,
+    buffer: Box<[UnsafeCell<MaybeUninit<task::Notified<T>>>; LOCAL_QUEUE_CAPACITY]>,


i guess the other question re https://github.com/tokio-rs/tokio/pull/3939/files#r667212885 is, does this even need to be boxed anymore, if it has a fixed size? should we save another layer of indirection by removing the Box, or is there a reason for this to live in a separate allocation beyond sizedness?

It doesn't need to, no. If I am to come up with a reason, then some allocators probably dislike allocations that are slightly larger than 1024 bytes. I don't know what effect this has in practice. I am open to removing the box.

I guess another reason is that long fixed size arrays are really awkward to construct for non-copy generic types.

runtime: move inject queue to `tokio::runtime::task` (tokio-rs#3939)

Darksonn added 3 commits July 7, 2021 11:55

runtime: move inject queue to task module

41599b5

Move task related unsafety into Inject::push_batch

e4ccb3a

Move length of box to the type system

d9f9133

Darksonn added A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime labels Jul 7, 2021

Darksonn added 2 commits July 7, 2021 16:31

Move inject to cfg_rt_multi_thread

8085daa

Remove unused trait bounds

e2d9825

carllerche approved these changes Jul 9, 2021

View reviewed changes

hawkw approved these changes Jul 9, 2021

View reviewed changes

hawkw reviewed Jul 9, 2021

View reviewed changes

fix nits

1b9932c

Darksonn merged commit 3b38ebd into master Jul 12, 2021

Darksonn deleted the safe-task branch July 12, 2021 08:31

sthagen added a commit to sthagen/tokio-rs-tokio that referenced this pull request Jul 12, 2021

Merge pull request #45 from tokio-rs/master

ea1b8da

runtime: move inject queue to `tokio::runtime::task` (tokio-rs#3939)

Darksonn mentioned this pull request Jul 16, 2021

Prepare Tokio v1.9.0 #3961

Merged

softdevca mentioned this pull request Jul 22, 2021

Bump tokio from 1.8.2 to 1.9.0 softdevca/mootranscode#27

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move inject queue to `tokio::runtime::task` #3939

Move inject queue to `tokio::runtime::task` #3939

Darksonn commented Jul 7, 2021

carllerche left a comment

hawkw left a comment

hawkw Jul 9, 2021

Darksonn Jul 9, 2021

hawkw Jul 9, 2021

hawkw Jul 9, 2021

Darksonn Jul 9, 2021

Darksonn Jul 9, 2021

Move inject queue to tokio::runtime::task #3939

Move inject queue to tokio::runtime::task #3939

Conversation

Darksonn commented Jul 7, 2021

carllerche left a comment

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw Jul 9, 2021

Choose a reason for hiding this comment

Darksonn Jul 9, 2021

Choose a reason for hiding this comment

hawkw Jul 9, 2021

Choose a reason for hiding this comment

hawkw Jul 9, 2021

Choose a reason for hiding this comment

Darksonn Jul 9, 2021

Choose a reason for hiding this comment

Darksonn Jul 9, 2021

Choose a reason for hiding this comment

Move inject queue to `tokio::runtime::task` #3939

Move inject queue to `tokio::runtime::task` #3939