Stream: git-wasmtime

Topic: wasmtime / PR #14625 wasi-http: expire requests whose cal...


view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 15:58):

xia-chao opened PR #14625 from xia-chao:fix/wasi-http-expire-dropped-request to bytecodealliance:main:

p3 had a TODO for a dropped receiver, so wired up poll_canceled. p2 can't use it, its sync guest blocks the event loop.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 15:58):

xia-chao requested rvolosatovs for a review on PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 15:58):

xia-chao requested wasmtime-wasi-reviewers for a review on PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 09 2026 at 16:08):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:memo: rvolosatovs submitted PR review:

Thanks!

I think we could simplify this by dropping the guard and instead relying on oneshot::Sender functionality directly, what do you think?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

Shouldn't this also wake the worker?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

in p3 the worker might be processing multiple requests concurrently, returning here would tear down the worker and abort processing requests that are currently in-process and prevent it from accepting more requests, I don't think that's desirable.

I think we should only return here on P2 (which we can do if we always leave the vector empty on P3)

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

We probably don't want this to grow without a bound

We should probably also check abandoned here early, since a client could have disconnected while the request was in the queue

Additionally, we should probably check if this is P2 (perhaps by checking &prepared?) and only push then

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

It seems that we could achieve the same in p2 with:

tx.lock().unwrap().as_mut().is_some_and(|tx| tx.poll_canceled(cx).is_ready())

If we stored clones of the Arc<Mutex<Option<Sender>>> instead of the atomic bool

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

It looks like there is a race condition here if handle future is dropped after the response is received, but before it's polled. Perhaps the simplest fix here could be storing the channel itself in the guard and doing close and try_recv on Drop?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

Do we actually need the abandoned? Couldn't we use tx and https://docs.rs/futures/latest/futures/channel/oneshot/struct.Sender.html#method.poll_canceled to check if the receiver has been dropped?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

                let closed = pin!(future::poll_fn(|cx| tx.poll_canceled(cx)));

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

                    handle,
                    futures::future::select(closed, expiration)
                )
                .await
                {
                    Either::Left((result, ..)) => (result, true),
                    Either::Right(_) => {
                        // TODO: We should also send a cancel request to the expired
                        // task to give it a chance to shut down gracefully, but as of
                        // this writing Wasmtime does not yet provide an API for doing
                        // that.  See issue #11833.  Instead, we let it continue running
                        // as a background task until it either returns a response
                        // (which we'll ignore) or the instance itself has expired.
                        (Err(ExpirationError.into()), false)
                    }

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 16:14):

:speech_balloon: rvolosatovs created PR review comment:

nit; could we use the poll_fn and future:: imports here?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:06):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:10):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:10):

:speech_balloon: xia-chao created PR review comment:

Thank you! agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:10):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:10):

:speech_balloon: xia-chao created PR review comment:

Thank you! agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:12):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:12):

:speech_balloon: xia-chao created PR review comment:

I only did 'push only on P2', I don't quite approve of the rest

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:12):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:12):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:13):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:13):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:13):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:13):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:13):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:13):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:15):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:15):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:15):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:15):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:28):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:47):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:47):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 17:47):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:01):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:04):

:speech_balloon: xia-chao edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:06):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:06):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:06):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:06):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:06):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 14 2026 at 18:06):

:speech_balloon: xia-chao created PR review comment:

Thank you! Not really agree

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 09:55):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 09:55):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 09:55):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:03):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:speech_balloon: xia-chao created PR review comment:

I don't think this needs a change. That drop happens after the response is already in the channel, and I didn't see it get lost.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:speech_balloon: xia-chao created PR review comment:

I kept the flag. poll_canceled would also see the drop, but this already wakes the outer poll.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:06):

:speech_balloon: xia-chao created PR review comment:

Same idea on p2. The flag and the wake already unblock that wait, so I didn't switch it over.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:10):

:speech_balloon: xia-chao edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:17):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:17):

:speech_balloon: xia-chao deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:18):

xia-chao requested rvolosatovs for a review on PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:50):

:repeat: rvolosatovs submitted PR review:

Thanks for the updates, unfortunately it seems that the PR still introduces 2 regressions, one in p2 and one in p3. See https://github.com/rvolosatovs/wasmtime/blob/review/pr-14625-repro/crates/wasi-http/tests/all/pr14625.rs for a reproducer

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 10:50):

:repeat: rvolosatovs submitted PR review:

Thanks for the updates, unfortunately it seems that the PR still introduces 2 regressions, one in p2 and one in p3. See https://github.com/rvolosatovs/wasmtime/blob/review/pr-14625-repro/crates/wasi-http/tests/all/pr14625.rs for a reproducer (which passes on main)

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 15:53):

xia-chao requested alexcrichton for a review on PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 15:53):

xia-chao requested wasmtime-core-reviewers for a review on PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 15:53):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 15:56):

xia-chao converted PR #14625 wasi-http: expire requests whose caller dropped the response to a draft.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 16:25):

xia-chao updated PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 15 2026 at 16:38):

xia-chao has marked PR #14625 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:10):

xia-chao requested rvolosatovs for a review on PR #14625.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 19 2026 at 18:16):

:speech_balloon: rvolosatovs created PR review comment:

I'm still concerned about this growing without a bound. If that's not an issue, could you add a comment explaining why?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 19 2026 at 18:16):

:memo: rvolosatovs submitted PR review:

Thanks for the updates!

See https://github.com/rvolosatovs/wasmtime/commit/fb49791fdae96cbc59c45e092f3d32417877e355

cargo test -p wasmtime-wasi-http --features component-model-async --test all handler

p3_caller_drop_keeps_request_expiration fails on this PR, but passes on main

This means that once a caller drops, its request's expiration is dropped too: the instance keeps accepting new requests even after request_timeout has passed, and the orphaned guest tasks accumulate indefinitely.

Could you take a look?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 10:54):

xia-chao commented on PR #14625:

I'm closing this PR.
I also suggest removing the TODO: it conflicts with the new tests,expiring on caller drop is the opposite of leaving the worker accepting.
@rvolosatovs

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 10:55):

:cross_mark: xia-chao closed without merge PR #14625.


Last updated: Oct 31 2026 at 23:03 UTC