Wasmtime GitHub notifications bot (Mar 13 2025 at 19:13): Wasmtime GitHub notifications bot (Mar 13 2025 at 19:13):alexcrichton requested wasmtime-core-reviewers for a review on PR #10394.
Wasmtime GitHub notifications bot (Mar 13 2025 at 19:13):alexcrichton requested dicej for a review on PR #10394.
Wasmtime GitHub notifications bot (Mar 13 2025 at 19:13):alexcrichton opened PR #10394 from alexcrichton:graceful-shutdown-serve to bytecodealliance:main:
This commit fixes the spurious CI failure found [here][fail]. The problem here is that the infrastructure for testing
wasmtime servewas not properly waiting for all output infinish. There's some infrastructure in the tests for spawning a subprocess and managing it, but prior to this commit there was no way to shut down the server gracefully and thus read all pending output from the child tasks.The specific problem is that a specific error message was expected in the logs after a request had been processed, but the
finishmethod wasn't reading the message. The reason for this is thatfinishhad to resort tokill -9on the child process as there was no other means of shutting it down. This meant that the print, which happened after request completion, might be killed and never happen.The solution ended up in this commit is to (a) add a
--shutdown-addrCLI flag towasmtime serveand (b) beef up handling of graceful shutdown. Previously ctrl-c was used to exit the server but it didn't do anything but drop all in-progress work. Instead graceful shutdown is now handled by breaking out of theacceptloop and then waiting for all child tasks to complete, meaning that no http requests once received are cancelled. In addition to ctrl-c the--shutdown-addris used to listen for a TCP connection which is a second means of cancellation. The reason for this is that sending ctrl-c to a process is not nearly as trivial on Windows as it is on Unix, so I didn't want to deal with all the console bits necessary to get that aligned.[fail]: https://github.com/bytecodealliance/wasmtime/actions/runs/13833291117/job/38702374924?pr=10390
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Mar 13 2025 at 19:28):dicej submitted PR review.
Wasmtime GitHub notifications bot (Mar 13 2025 at 19:28):dicej created PR review comment:
Looks like this was included by mistake?
Wasmtime GitHub notifications bot (Mar 13 2025 at 19:28):dicej created PR review comment:
It doesn't look like we send a ctrl-c here, regardless of platform, so maybe change this comment to read "...so connect to the shutdown port..." or something similar.
Wasmtime GitHub notifications bot (Mar 13 2025 at 23:07):alexcrichton updated PR #10394.
Wasmtime GitHub notifications bot (Mar 13 2025 at 23:07):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Mar 13 2025 at 23:07):alexcrichton created PR review comment:
Oops sorry I was halfway through updating this comment when I context switched at some point and forgot to update the other half of the comment...
Wasmtime GitHub notifications bot (Mar 13 2025 at 23:07):alexcrichton has enabled auto merge for PR #10394.
Wasmtime GitHub notifications bot (Mar 13 2025 at 23:42):alexcrichton merged PR #10394.
Last updated: Apr 18 2025 at 18:04 UTC