rt: do not track time-based metrics on wasm32-unknown-unknown by jasgin · Pull Request #7322 · tokio-rs/tokio

jasgin · 2025-05-08T15:29:18Z

Fix for #7319

Time is generally unsupported in WASM. Fix involves removing Instant from metrics handling when WASM is the target family and tokio's time feature is not enabled.

NOTES:
Will iterate with a test to catch this in the future.

rcoh · 2025-05-08T15:44:53Z


+    #[cfg(target_family = "wasm")]
+    pub(crate) fn new_for_wasm() -> MetricsBatch {
+        Self::new_unstable_for_wasm()


what do you think instead about a function like:

fn now() -> Option<Instant> { if cfg!(target_family = "wasm") { None } else { Instant::now() } }

Then use that everywhere that its needed? Since WASM can compile the now function, it just can't call it, I think this should more or less take care of it

Good idea, I'm on board with that

rcoh · 2025-05-08T15:45:39Z

+        if let Some(processing_scheduled_tasks_started_at) =
+            self.processing_scheduled_tasks_started_at
+        {
+            let busy_duration = processing_scheduled_tasks_started_at.elapsed();


I like the impact of making this optional since it ensures we can't call elapsed() unless we actually got an instant.

rcoh · 2025-05-08T18:06:30Z

            #[inline(always)]
-            fn new_unstable(worker_metrics: &WorkerMetrics, now: Instant) -> MetricsBatch {
+            fn new_unstable(worker_metrics: &WorkerMetrics, maybe_now: Option<Instant>) -> MetricsBatch {
+                let poll_timer = maybe_now.map_or(None, |now| {


ah right, I guess and_then due to the extra option?

rcoh · 2025-05-08T18:07:09Z

    u64::try_from(dur.as_nanos()).unwrap_or(u64::MAX)
 }
+
+fn now() -> Option<Instant> {


document why this exists

rcoh · 2025-05-08T18:07:17Z

+        if self.processing_scheduled_tasks_started_at.is_some() {
+            self.processing_scheduled_tasks_started_at = Some(Instant::now());
+        }
    }


this can use now()

time is unsupported in WASM, fix involves removing Instant from metrics handling for cfg gated WASM target family as a stopgap solution

Darksonn · 2025-05-09T09:43:15Z

    u64::try_from(dur.as_nanos()).unwrap_or(u64::MAX)
 }
+
+/// `WASM` does not support time: <https://github.com/tokio-rs/tokio/issues/7319>


The major problem here is that WASM does support time ... sometimes. Perhaps we could change this logic to only apply when you enable the time feature of Tokio? I'm not sure if that would be breaking.

That seems reasonable. In the longer term, we could add a builder option to opt in or out of these time based metrics.

Thanks. Added this, let me know if we wanted to conditionally compile the fields as well, but it seems viable to leave them as None when time is not enabled (esp if it's a temporary fix).

rcoh

LGTM — This sounds like a good stopgap to unbreak people who are broken by the current release. Not sure what the right long term option is.

Darksonn · 2025-05-10T17:32:50Z

@jccampagne does this work for you?

jccampagne · 2025-05-13T09:58:42Z

Hi, thanks for the reply; I am offline for a few days; will try later in the week.

jccampagne · 2025-05-16T07:02:58Z

I just tested it - it works for me.
Thanks everyone!

Darksonn · 2025-05-21T23:42:14Z

Sorry, I still have a question about this. Does this mean that we still are making a breaking change in this scenario:

Using WASM on a platform without time.
Time feature is enabled.

After all, I think but am not sure that this works today as long as you don't actually use tokio::time::sleep. But I could be mistaken.

rcoh · 2025-05-22T00:45:51Z

This change is very narrow, it's only for metrics.

These metrics could never have worked since they call Instant::now.

The set of people broken by this change is:

on WASM
platform supports Instant::now
they don't have the time feature enabled

These customers would previously have non zero values for these metrics but now they don't.

However, these metrics were only stabilized in the last release.

Darksonn · 2025-05-22T04:07:26Z

I don't mean broken just by this change since last release, but broken since the release before that - after all, that's the breakage this is trying to fix.

rcoh · 2025-05-23T10:33:14Z

oh yeah I see what you're saying.

Unfortunately this change fully solves neither:

People broken by the inclusion of Instant::now in metrics (because enabling time is only a weak heuristic for time working)
People seeing 0 for these metrics who could be seeing them (because there's no reason why you need to enable time just because your platform supports time)

Ultimately the only thing that would actually work is:

A cfg gate provided by rustc that reliably determines whether Instant::now will work
A standard library method like Instant::try_now that does this.

I started a discussion to this effect here.

Noratrieb · 2025-05-23T13:47:36Z

+/// Gate time metrics as platforms like `WASM` do not typically support [`std::time`]
+/// <https://github.com/tokio-rs/tokio/issues/7319>
+fn now() -> Option<Instant> {
+    if cfg!(target_family = "wasm") && !cfg!(feature = "time") {


You should do an exact match: cfg!(all(target_arch = "wasm", target_os = "unknown", target_vendor = "unknown")). Instant::now should be available just fine on all other Wasm targets that have std.

It is not expected that any new broken targets like this will be added.

Darksonn

Thank you. Now working on merging this to tokio-1.45.x for a patch release.

The time feature is not relevant here. WASM with time enabled should wokr on wasm32-unknown-unknown as long as you don't actually call tokio::time::sleep.

Bumps tokio from 1.45.0 to 1.45.1. Release notes Sourced from tokio's releases. Tokio v1.45.1 1.45.1 (May 24th, 2025) This fixes a regression on the wasm32-unknown-unknown target, where code that previously did not panic due to calls to Instant::now() started failing. This is due to the stabilization of the first time-based metric. Fixed Disable time-based metrics on wasm32-unknown-unknown (#7322) #7322: tokio-rs/tokio#7322 Commits 3768696 chore: prepare Tokio v1.45.1 (#7359) 421a7b0 rt: do not track time-based metrics on wasm32-unknown-unknown (#7322) b1bdb3c ci: update macros_type_mismatch for Rust 1.87.0 (#7339) See full diff in compare view Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase. Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: @dependabot rebase will rebase this PR @dependabot recreate will recreate this PR, overwriting any edits that have been made to it @dependabot merge will merge this PR after your CI passes on it @dependabot squash and merge will squash and merge this PR after your CI passes on it @dependabot cancel merge will cancel a previously requested merge and block automerging @dependabot reopen will reopen this PR if it is closed @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

soundofspace and others added 2 commits May 6, 2025 17:27

net: ignore NotConnected in TcpStream::shutdown (tokio-rs#7290)

f0fdef8

runtime: add doc note that on_*_task_poll is unstable (tokio-rs#7311)

bdd64cc

github-actions Bot added the R-loom-current-thread Run loom current-thread tests on this PR label May 8, 2025

jasgin force-pushed the fix-time-for-wasm branch from f4e99fa to 1fe2ebe Compare May 8, 2025 15:32

rcoh reviewed May 8, 2025

View reviewed changes

jasgin force-pushed the fix-time-for-wasm branch from 1fe2ebe to b93f58e Compare May 8, 2025 18:01

github-actions Bot removed the R-loom-current-thread Run loom current-thread tests on this PR label May 8, 2025

jasgin force-pushed the fix-time-for-wasm branch from b93f58e to 9754ded Compare May 8, 2025 18:03

rcoh reviewed May 8, 2025

View reviewed changes

jasgin force-pushed the fix-time-for-wasm branch 3 times, most recently from a6c8891 to 266709e Compare May 8, 2025 18:38

rt: fix wasm target by removing time from metrics

fd50424

time is unsupported in WASM, fix involves removing Instant from metrics handling for cfg gated WASM target family as a stopgap solution

jasgin force-pushed the fix-time-for-wasm branch from 266709e to fd50424 Compare May 8, 2025 19:12

Darksonn added T-docs Topic: documentation A-tokio Area: The main tokio crate M-time Module: tokio/time M-metrics Module: tokio/runtime/metrics T-wasm Topic: Web Assembly and removed T-docs Topic: documentation labels May 9, 2025

Darksonn reviewed May 9, 2025

View reviewed changes

jasgin force-pushed the fix-time-for-wasm branch 3 times, most recently from 017d1ad to 71b5702 Compare May 9, 2025 16:53

rt: make the fix also gated behind time feature being disabled

2a1d0ad

jasgin force-pushed the fix-time-for-wasm branch from 71b5702 to 2a1d0ad Compare May 9, 2025 17:00

rcoh approved these changes May 9, 2025

View reviewed changes

Noratrieb reviewed May 23, 2025

View reviewed changes

Jason Gin and others added 2 commits May 23, 2025 15:34

rt: use exact match for wasm32-unknown-unknown

017ce9e

Merge branch 'tokio-1.45.x' into 'fix-time-for-wasm'

0a0ee85

Darksonn force-pushed the fix-time-for-wasm branch from 34a5d95 to 0a0ee85 Compare May 23, 2025 17:52

Darksonn changed the base branch from master to tokio-1.45.x May 23, 2025 17:53

Darksonn approved these changes May 23, 2025

View reviewed changes

Darksonn changed the title ~~rt: fix wasm target by removing time from metrics~~ rt: do not track time-based metrics on wasm32-unknown-unknown May 23, 2025

Remove time feature from cfg! logic

e77913a

The time feature is not relevant here. WASM with time enabled should wokr on wasm32-unknown-unknown as long as you don't actually call tokio::time::sleep.

Darksonn enabled auto-merge (squash) May 23, 2025 18:54

Darksonn merged commit 421a7b0 into tokio-rs:tokio-1.45.x May 23, 2025
80 checks passed

This was referenced May 24, 2025

chore: prepare Tokio v1.45.1 #7359

Merged

Tokio 1.45.0 time not implemented on this platform on WASM (target.wasm32-unknown-unknown) #7319

Closed

cratelyn mentioned this pull request May 28, 2025

build(deps): bump tokio from 1.45.0 to 1.45.1 linkerd/linkerd2-proxy#3935

Closed

Uh oh!

Conversation

jasgin commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rcoh left a comment

Choose a reason for hiding this comment

Uh oh!

Darksonn commented May 10, 2025

Uh oh!

jccampagne commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jccampagne commented May 16, 2025

Uh oh!

Darksonn commented May 21, 2025

Uh oh!

rcoh commented May 22, 2025

Uh oh!

Darksonn commented May 22, 2025

Uh oh!

rcoh commented May 23, 2025

Uh oh!

Noratrieb May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darksonn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jasgin commented May 8, 2025 •

edited

Loading

jccampagne commented May 13, 2025 •

edited

Loading

Noratrieb May 23, 2025 •

edited

Loading