Skip to content

Commit a0d5b8a

Browse files
authored
runtime(unstable): fix task hook spawn locations for tokio::spawn (#7440)
## Motivation Unfortunately, due to an oversight on my part, the capturing of spawn locations was only tested with the `Runtime::spawn` method, and *not* with `tokio::spawn`/`tokio::task::spawn`, which is how most tasks are spawned in Real Life. And, it turned out that because this was not tested...well, it was broken. Agh. My bad. ## Solution Although the whole call chain for spawning tasks using `tokio::spawn` was correctly annotated with `#[track_caller]`, the location wasn't propagated correctly because of the `context::with_current(|handle| { ... })` closure that accesses the current runtime. Because the call to spawn the task occurs inside a closure, the *closure*'s location is captured instead of the caller. This means any task spawned by `tokio::spawn` records its location as being in `tokio/src/task/spawn.rs`, which is not what we'd like. This commit fixes that by capturing the spawn location outside the `with_current` closure and passing it in explicitly. I've updated the tests to also spawn a task with `tokio::spawn`, so that we ensure this works correctly.
1 parent a1ee3ef commit a0d5b8a

File tree

11 files changed

+71
-63
lines changed

11 files changed

+71
-63
lines changed

tokio/src/runtime/handle.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ impl Handle {
329329
}
330330

331331
#[track_caller]
332-
pub(crate) fn spawn_named<F>(&self, future: F, _meta: SpawnMeta<'_>) -> JoinHandle<F::Output>
332+
pub(crate) fn spawn_named<F>(&self, future: F, meta: SpawnMeta<'_>) -> JoinHandle<F::Output>
333333
where
334334
F: Future + Send + 'static,
335335
F::Output: Send + 'static,
@@ -344,16 +344,16 @@ impl Handle {
344344
))]
345345
let future = super::task::trace::Trace::root(future);
346346
#[cfg(all(tokio_unstable, feature = "tracing"))]
347-
let future = crate::util::trace::task(future, "task", _meta, id.as_u64());
348-
self.inner.spawn(future, id)
347+
let future = crate::util::trace::task(future, "task", meta, id.as_u64());
348+
self.inner.spawn(future, id, meta.spawned_at)
349349
}
350350

351351
#[track_caller]
352352
#[allow(dead_code)]
353353
pub(crate) unsafe fn spawn_local_named<F>(
354354
&self,
355355
future: F,
356-
_meta: SpawnMeta<'_>,
356+
meta: SpawnMeta<'_>,
357357
) -> JoinHandle<F::Output>
358358
where
359359
F: Future + 'static,
@@ -369,8 +369,8 @@ impl Handle {
369369
))]
370370
let future = super::task::trace::Trace::root(future);
371371
#[cfg(all(tokio_unstable, feature = "tracing"))]
372-
let future = crate::util::trace::task(future, "task", _meta, id.as_u64());
373-
self.inner.spawn_local(future, id)
372+
let future = crate::util::trace::task(future, "task", meta, id.as_u64());
373+
self.inner.spawn_local(future, id, meta.spawned_at)
374374
}
375375

376376
/// Returns the flavor of the current `Runtime`.

tokio/src/runtime/scheduler/current_thread/mod.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::loom::sync::Arc;
33
use crate::runtime::driver::{self, Driver};
44
use crate::runtime::scheduler::{self, Defer, Inject};
55
use crate::runtime::task::{
6-
self, JoinHandle, OwnedTasks, Schedule, Task, TaskHarnessScheduleHooks,
6+
self, JoinHandle, OwnedTasks, Schedule, SpawnLocation, Task, TaskHarnessScheduleHooks,
77
};
88
use crate::runtime::{
99
blocking, context, Config, MetricsBatch, SchedulerMetrics, TaskHooks, TaskMeta, WorkerMetrics,
@@ -15,7 +15,6 @@ use crate::util::{waker_ref, RngSeedGenerator, Wake, WakerRef};
1515
use std::cell::RefCell;
1616
use std::collections::VecDeque;
1717
use std::future::{poll_fn, Future};
18-
use std::panic::Location;
1918
use std::sync::atomic::Ordering::{AcqRel, Release};
2019
use std::task::Poll::{Pending, Ready};
2120
use std::task::Waker;
@@ -451,16 +450,13 @@ impl Handle {
451450
me: &Arc<Self>,
452451
future: F,
453452
id: crate::runtime::task::Id,
453+
spawned_at: SpawnLocation,
454454
) -> JoinHandle<F::Output>
455455
where
456456
F: crate::future::Future + Send + 'static,
457457
F::Output: Send + 'static,
458458
{
459-
let spawned_at = Location::caller();
460-
let (handle, notified) = me
461-
.shared
462-
.owned
463-
.bind(future, me.clone(), id, spawned_at.into());
459+
let (handle, notified) = me.shared.owned.bind(future, me.clone(), id, spawned_at);
464460

465461
me.task_hooks.spawn(&TaskMeta {
466462
id,
@@ -486,16 +482,16 @@ impl Handle {
486482
me: &Arc<Self>,
487483
future: F,
488484
id: crate::runtime::task::Id,
485+
spawned_at: SpawnLocation,
489486
) -> JoinHandle<F::Output>
490487
where
491488
F: crate::future::Future + 'static,
492489
F::Output: 'static,
493490
{
494-
let spawned_at = Location::caller();
495-
let (handle, notified) =
496-
me.shared
497-
.owned
498-
.bind_local(future, me.clone(), id, spawned_at.into());
491+
let (handle, notified) = me
492+
.shared
493+
.owned
494+
.bind_local(future, me.clone(), id, spawned_at);
499495

500496
me.task_hooks.spawn(&TaskMeta {
501497
id,

tokio/src/runtime/scheduler/mod.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl Handle {
6868
cfg_rt! {
6969
use crate::future::Future;
7070
use crate::loom::sync::Arc;
71-
use crate::runtime::{blocking, task::Id};
71+
use crate::runtime::{blocking, task::{Id, SpawnLocation}};
7272
use crate::runtime::context;
7373
use crate::task::JoinHandle;
7474
use crate::util::RngSeedGenerator;
@@ -117,17 +117,16 @@ cfg_rt! {
117117
}
118118
}
119119

120-
#[track_caller]
121-
pub(crate) fn spawn<F>(&self, future: F, id: Id) -> JoinHandle<F::Output>
120+
pub(crate) fn spawn<F>(&self, future: F, id: Id, spawned_at: SpawnLocation) -> JoinHandle<F::Output>
122121
where
123122
F: Future + Send + 'static,
124123
F::Output: Send + 'static,
125124
{
126125
match self {
127-
Handle::CurrentThread(h) => current_thread::Handle::spawn(h, future, id),
126+
Handle::CurrentThread(h) => current_thread::Handle::spawn(h, future, id, spawned_at),
128127

129128
#[cfg(feature = "rt-multi-thread")]
130-
Handle::MultiThread(h) => multi_thread::Handle::spawn(h, future, id),
129+
Handle::MultiThread(h) => multi_thread::Handle::spawn(h, future, id, spawned_at),
131130
}
132131
}
133132

@@ -138,13 +137,13 @@ cfg_rt! {
138137
/// by the current thread.
139138
#[allow(irrefutable_let_patterns)]
140139
#[track_caller]
141-
pub(crate) unsafe fn spawn_local<F>(&self, future: F, id: Id) -> JoinHandle<F::Output>
140+
pub(crate) unsafe fn spawn_local<F>(&self, future: F, id: Id, spawned_at: SpawnLocation) -> JoinHandle<F::Output>
142141
where
143142
F: Future + 'static,
144143
F::Output: 'static,
145144
{
146145
if let Handle::CurrentThread(h) = self {
147-
current_thread::Handle::spawn_local(h, future, id)
146+
current_thread::Handle::spawn_local(h, future, id, spawned_at)
148147
} else {
149148
panic!("Only current_thread and LocalSet have spawn_local internals implemented")
150149
}

tokio/src/runtime/scheduler/multi_thread/handle.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ use crate::runtime::scheduler::multi_thread::worker;
44
use crate::runtime::task::{Notified, Task, TaskHarnessScheduleHooks};
55
use crate::runtime::{
66
blocking, driver,
7-
task::{self, JoinHandle},
7+
task::{self, JoinHandle, SpawnLocation},
88
TaskHooks, TaskMeta,
99
};
1010
use crate::util::RngSeedGenerator;
1111

1212
use std::fmt;
13-
use std::panic::Location;
1413

1514
mod metrics;
1615

@@ -38,30 +37,35 @@ pub(crate) struct Handle {
3837

3938
impl Handle {
4039
/// Spawns a future onto the thread pool
41-
#[track_caller]
42-
pub(crate) fn spawn<F>(me: &Arc<Self>, future: F, id: task::Id) -> JoinHandle<F::Output>
40+
pub(crate) fn spawn<F>(
41+
me: &Arc<Self>,
42+
future: F,
43+
id: task::Id,
44+
spawned_at: SpawnLocation,
45+
) -> JoinHandle<F::Output>
4346
where
4447
F: crate::future::Future + Send + 'static,
4548
F::Output: Send + 'static,
4649
{
47-
Self::bind_new_task(me, future, id)
50+
Self::bind_new_task(me, future, id, spawned_at)
4851
}
4952

5053
pub(crate) fn shutdown(&self) {
5154
self.close();
5255
}
5356

5457
#[track_caller]
55-
pub(super) fn bind_new_task<T>(me: &Arc<Self>, future: T, id: task::Id) -> JoinHandle<T::Output>
58+
pub(super) fn bind_new_task<T>(
59+
me: &Arc<Self>,
60+
future: T,
61+
id: task::Id,
62+
spawned_at: SpawnLocation,
63+
) -> JoinHandle<T::Output>
5664
where
5765
T: Future + Send + 'static,
5866
T::Output: Send + 'static,
5967
{
60-
let spawned_at = Location::caller();
61-
let (handle, notified) = me
62-
.shared
63-
.owned
64-
.bind(future, me.clone(), id, spawned_at.into());
68+
let (handle, notified) = me.shared.owned.bind(future, me.clone(), id, spawned_at);
6569

6670
me.task_hooks.spawn(&TaskMeta {
6771
id,

tokio/src/runtime/task/harness.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ where
373373
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| {
374374
f(&TaskMeta {
375375
id: self.core().task_id,
376-
spawned_at: self.core().spawned_at,
376+
spawned_at: self.core().spawned_at.into(),
377377
_phantom: Default::default(),
378378
})
379379
}));

tokio/src/runtime/task/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ impl<S: 'static> Task<S> {
444444
pub(crate) fn task_meta<'task, 'meta>(&'task self) -> crate::runtime::TaskMeta<'meta> {
445445
crate::runtime::TaskMeta {
446446
id: self.id(),
447-
spawned_at: self.spawned_at(),
447+
spawned_at: self.spawned_at().into(),
448448
_phantom: PhantomData,
449449
}
450450
}

tokio/src/runtime/task_hooks.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
use std::marker::PhantomData;
2-
use std::panic::Location;
3-
41
use super::Config;
2+
use std::marker::PhantomData;
53

64
impl TaskHooks {
75
pub(crate) fn spawn(&self, meta: &TaskMeta<'_>) {
@@ -62,7 +60,8 @@ pub struct TaskMeta<'a> {
6260
/// The opaque ID of the task.
6361
pub(crate) id: super::task::Id,
6462
/// The location where the task was spawned.
65-
pub(crate) spawned_at: &'static Location<'static>,
63+
#[cfg_attr(not(tokio_unstable), allow(unreachable_pub, dead_code))]
64+
pub(crate) spawned_at: crate::runtime::task::SpawnLocation,
6665
pub(crate) _phantom: PhantomData<&'a ()>,
6766
}
6867

@@ -74,9 +73,9 @@ impl<'a> TaskMeta<'a> {
7473
}
7574

7675
/// Return the source code location where the task was spawned.
77-
#[cfg_attr(not(tokio_unstable), allow(unreachable_pub, dead_code))]
78-
pub fn spawned_at(&self) -> &'static Location<'static> {
79-
self.spawned_at
76+
#[cfg(tokio_unstable)]
77+
pub fn spawned_at(&self) -> &'static std::panic::Location<'static> {
78+
self.spawned_at.0
8079
}
8180
}
8281

tokio/src/task/local.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ cfg_rt! {
414414
let task = crate::util::trace::task(future, "task", meta, id.as_u64());
415415

416416
// safety: we have verified that this is a `LocalRuntime` owned by the current thread
417-
unsafe { handle.spawn_local(task, id) }
417+
unsafe { handle.spawn_local(task, id, meta.spawned_at) }
418418
} else {
419419
match CURRENT.with(|LocalData { ctx, .. }| ctx.get()) {
420420
None => panic!("`spawn_local` called from outside of a `task::LocalSet` or LocalRuntime"),

tokio/src/task/spawn.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ cfg_rt! {
199199
let id = task::Id::next();
200200
let task = crate::util::trace::task(future, "task", meta, id.as_u64());
201201

202-
match context::with_current(|handle| handle.spawn(task, id)) {
202+
match context::with_current(|handle| handle.spawn(task, id, meta.spawned_at)) {
203203
Ok(join_handle) => join_handle,
204204
Err(e) => panic!("{}", e),
205205
}

tokio/src/util/trace.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,29 @@ cfg_rt! {
99
/// The original size of the future or function being spawned
1010
#[cfg(all(tokio_unstable, feature = "tracing"))]
1111
pub(crate) original_size: usize,
12+
/// The source code location where the task was spawned.
13+
///
14+
/// This is wrapped in a type that may be empty when `tokio_unstable` is
15+
/// not enabled.
16+
pub(crate) spawned_at: crate::runtime::task::SpawnLocation,
1217
_pd: PhantomData<&'a ()>,
1318
}
1419

1520
impl<'a> SpawnMeta<'a> {
1621
/// Create new spawn meta with a name and original size (before possible auto-boxing)
1722
#[cfg(all(tokio_unstable, feature = "tracing"))]
23+
#[track_caller]
1824
pub(crate) fn new(name: Option<&'a str>, original_size: usize) -> Self {
1925
Self {
2026
name,
2127
original_size,
28+
spawned_at: crate::runtime::task::SpawnLocation::capture(),
2229
_pd: PhantomData,
2330
}
2431
}
2532

2633
/// Create a new unnamed spawn meta with the original size (before possible auto-boxing)
34+
#[track_caller]
2735
pub(crate) fn new_unnamed(original_size: usize) -> Self {
2836
#[cfg(not(all(tokio_unstable, feature = "tracing")))]
2937
let _original_size = original_size;
@@ -33,6 +41,7 @@ cfg_rt! {
3341
name: None,
3442
#[cfg(all(tokio_unstable, feature = "tracing"))]
3543
original_size,
44+
spawned_at: crate::runtime::task::SpawnLocation::capture(),
3645
_pd: PhantomData,
3746
}
3847
}
@@ -50,11 +59,8 @@ cfg_rt! {
5059
pub(crate) use tracing::instrument::Instrumented;
5160

5261
#[inline]
53-
#[track_caller]
5462
pub(crate) fn task<F>(task: F, kind: &'static str, meta: SpawnMeta<'_>, id: u64) -> Instrumented<F> {
55-
#[track_caller]
5663
fn get_span(kind: &'static str, spawn_meta: SpawnMeta<'_>, id: u64, task_size: usize) -> tracing::Span {
57-
let location = std::panic::Location::caller();
5864
let original_size = if spawn_meta.original_size != task_size {
5965
Some(spawn_meta.original_size)
6066
} else {
@@ -69,9 +75,9 @@ cfg_rt! {
6975
task.id = id,
7076
original_size.bytes = original_size,
7177
size.bytes = task_size,
72-
loc.file = location.file(),
73-
loc.line = location.line(),
74-
loc.col = location.column(),
78+
loc.file = spawn_meta.spawned_at.0.file(),
79+
loc.line = spawn_meta.spawned_at.0.line(),
80+
loc.col = spawn_meta.spawned_at.0.column(),
7581
)
7682
}
7783
use tracing::instrument::Instrument;
@@ -80,10 +86,7 @@ cfg_rt! {
8086
}
8187

8288
#[inline]
83-
#[track_caller]
8489
pub(crate) fn blocking_task<Fn, Fut>(task: Fut, spawn_meta: SpawnMeta<'_>, id: u64) -> Instrumented<Fut> {
85-
let location = std::panic::Location::caller();
86-
8790
let fn_size = mem::size_of::<Fn>();
8891
let original_size = if spawn_meta.original_size != fn_size {
8992
Some(spawn_meta.original_size)
@@ -100,9 +103,9 @@ cfg_rt! {
100103
"fn" = %std::any::type_name::<Fn>(),
101104
original_size.bytes = original_size,
102105
size.bytes = fn_size,
103-
loc.file = location.file(),
104-
loc.line = location.line(),
105-
loc.col = location.column(),
106+
loc.file = spawn_meta.spawned_at.0.file(),
107+
loc.line = spawn_meta.spawned_at.0.line(),
108+
loc.col = spawn_meta.spawned_at.0.column(),
106109
);
107110
task.instrument(span)
108111

0 commit comments

Comments
 (0)