Skip to content

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jul 2, 2025

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.

hawkw added 2 commits July 2, 2025 12:22
Due to an oversignt on my part, we were only testing that spawn
locations were correct with the `Runtime::spawn` method, and not with
the `tokio::spawn` free function. And it turns out that was broken.
Great job, Eliza. :/
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.

Although the whole call chain for spawning tasks using `tokio::spawn`
was correctly annoted 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.
@hawkw hawkw requested review from hds, Darksonn and ADD-SP July 2, 2025 19:27
@github-actions github-actions bot added R-loom-current-thread Run loom current-thread tests on this PR R-loom-multi-thread Run loom multi-thread tests on this PR R-loom-multi-thread-alt labels Jul 2, 2025
@hds
Copy link
Contributor

hds commented Jul 2, 2025

@hawkw How about we include tests for these cases using tracing-mock as well?

This provides us with pretty clear integrations tests of this functionality.

I had only covered one case up until now (for the spawn location), but adding more should be easy enough. Like this one:

https://github.com/tokio-rs/tokio/blob/master/tokio/tests/tracing_task.rs#L79

@hawkw
Copy link
Member Author

hawkw commented Jul 2, 2025

@hds I'd be happy to add tests for the tracing integration as well! However, that was previously using a different code path and wouldn't actually have caught this issue --- the locations for tracing spans were being captured in a different place than where the one that's put in the task for the runtime hooks. Now, we're using the same code path for both, which is the only reason the tracing bits were touched in this branch.

Do you have a preference on whether to add those tests in this PR or a separate one?

@hds
Copy link
Contributor

hds commented Jul 3, 2025

In this case I think that another PR works fine.

Copy link
Contributor

@hds hds left a comment

Choose a reason for hiding this comment

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

Looks good to me! Funnily enough, I'd forgotten about the spawn meta in there.

@hawkw hawkw merged commit a0d5b8a into master Jul 4, 2025
87 checks passed
@hawkw hawkw deleted the eliza/fix-spawn-locations branch July 4, 2025 16:25
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit that referenced this pull request Jul 4, 2025
# 1.46.1 (July 4th, 2025)

This release fixes incorrect spawn locations in runtime task hooks for tasks
spawned using `tokio::spawn` rather than `Runtime::spawn`. This issue only
effected the spawn location in `TaskMeta::spawned_at`, and did not effect task
locations in Tracing events.

## Unstable

- runtime: add `TaskMeta::spawn_location` tracking where a task was spawned
  ([#7440)])

[#7440]: #7440
hawkw added a commit to oxidecomputer/tokio-dtrace that referenced this pull request Jul 4, 2025
Tokio PRs tokio-rs/tokio#7417 and tokio-rs/tokio#7440 add the source
code locations at which a task was spawned to the `TaskMeta` structure
provided to the runtime task hook callbacks. These will give us the
ability to associate task IDs (which identify a task at runtime) with a
source code location in the instrumented program, which is going to make
everything a lot easier. This PR picks up those changes and updates the
minimum Tokio dependency to 1.46.1.

Closes #1
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime labels Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime R-loom-current-thread Run loom current-thread tests on this PR R-loom-multi-thread Run loom multi-thread tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants