Add windows support on async #290

jprendes · 2025-03-04T19:38:33Z

This PR adds Windows support to the async feature.
It introduces a transport layer with structs:

Socket: a thin wrapper around a boxed AsyncRead + AsyncWrite.
Listener: a thin wrapper around a boxed stream of Sockets

These struct have implementations for: uds, vsocks, and named pipes.

In the process, this PR also introduces some breaking change in the public API.

Client::connect is now async.
Client::new now takes a Socket instead of a RawFd.
- The old behaviour is preserved in Client::from_raw_unix_listener_fd, which takes a RawFd and is marked unsafe.
Connection::new now takes a Socket instead of any AsyncRead + AsyncWrite
Server::add_listener now takes a Listener instead of a RawFd.
Server::set_domain_{unix,vsock} have been removed.
- The old behavior is somewhat preserved in Client::add_{unix_vsock}_listener, which takes a RawFd and is marked unsafe.
Server doesn't implement AsRawFd or FromRawFd anymore.
TtrpcContext doesn't have an fd field anymore.

I think it would be possible to add Windows support without breaking the public API, but I believe the proposed changes make more sense while still providing an upgrade path.

src/asynchronous/transport.rs

jprendes · 2025-03-05T13:53:40Z

@Tim-Zhang PTAL

Add support for windows named pipes to the async codebase. This refactor the codes to use a transport abstraction instead of file descriptors. Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Fix failure with deny workflow in CI. This requires bumping cargo-deny-action, which introduces a breaking change in the syntax of deny.toml Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

jsturtevant

I tried it out on windows and it works!

src/asynchronous/server.rs

src/asynchronous/transport/windows.rs

The windows API for querying the named pipe path explicitly mentions that it shouldn't be used with pipes. To avoid that, store the path of the pipe when binging it, and reuse that value. Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Improve the error message when a server is started with no listener specified Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Tim-Zhang · 2025-03-06T09:38:03Z

@jprendes Thank you for your PR. The community has been waiting for this feature for a long time.
I am a bit busy these two weeks. @jsturtevant @wllenyj @lifupan @teawater Please take a look at this. thanks.

jsturtevant

I took a look at the windows implementation and tried it out. The tests have also been enabled to run all the examples on Windows too (Thanks @jprendes!)

The rest of the refactoring look like an overall improvement to me as well but let the other maintainers decide on the changes there.

LGTM

wllenyj

LGTM, Thanks!

example/utils.rs

jprendes · 2025-03-19T14:49:40Z

Gentle ping :-) @Tim-Zhang
Please let me know if I can help moving this forward.

jsturtevant · 2025-03-24T19:45:55Z

@Tim-Zhang I think we are good here and is ready for your final approval

src/asynchronous/transport.rs

Tim-Zhang · 2025-03-28T15:28:09Z

Hi @jprendes I tried to run the example async-stream with the command cargo run --example async-stream-client and I got the error:

called `Result::unwrap()` on an `Err` value: Others("Socket::connect error 系统找不到指定的文件。 (os error 2)")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\examples\async-stream-client.exe` (exit code: 101)

Is this expected behavior? Does this PR currently support async streams?

jprendes · 2025-03-28T15:35:36Z

That's odd.
I run this on one terminal

cargo run --example async-stream-server

and this on another terminal

cargo run --example async-stream-client

and I get

cargo run --example async-stream-client
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.63s
     Running `target\debug\examples\async-stream-client.exe`
***** Async Stream test is OK! *****

jprendes · 2025-03-28T15:51:09Z

I just noticed that I never enabled the async examples for windows in run_example, I'll push that

This commit enables on Windows the async examples in the run_examples test Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

jprendes · 2025-03-28T16:07:44Z

Looks like the streaming client is also working in CI
https://github.com/containerd/ttrpc-rust/actions/runs/14132937711/job/39597617179?pr=290#step:3:406

Tim-Zhang · 2025-03-28T16:23:18Z

Hi @jprendes I tried to run the example async-stream with the command cargo run --example async-stream-client and I got the error:
called `Result::unwrap()` on an `Err` value: Others("Socket::connect error 系统找不到指定的文件。 (os error 2)")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\examples\async-stream-client.exe` (exit code: 101)
Is this expected behavior? Does this PR currently support async streams?

@jprendes Sorry, I think it was a wrong bug report. Forget it please.

since transport.rs has submodules, move it from transport.rs to transport/mod.rs Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Tim-Zhang

LGTM, Thanks @jprendes

Bump crate versions for publishing. The compiler crate needs a major version bump after breaking changes introduced by PR containerd#291 The main crate needs a major version bump after the braking changes introduced by PR containerd#290 Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Bump crate versions for publishing. The compiler crate needs a major version bump after breaking changes introduced by PR containerd#291 The codegen crate needs a major version bump because it exports an item from the compiler crate. The main crate needs a major version bump after the braking changes introduced by PR containerd#290 Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Add windows support on async

Bump crate versions for publishing. The compiler crate needs a major version bump after breaking changes introduced by PR containerd#291 The codegen crate needs a major version bump because it exports an item from the compiler crate. The main crate needs a major version bump after the braking changes introduced by PR containerd#290 Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

jsturtevant reviewed Mar 4, 2025

View reviewed changes

src/asynchronous/transport.rs Outdated Show resolved Hide resolved

jprendes force-pushed the async-windows branch 11 times, most recently from 1641dbf to 2c3e9d0 Compare March 5, 2025 12:51

jprendes marked this pull request as ready for review March 5, 2025 13:09

jprendes requested a review from jsturtevant March 5, 2025 13:53

jprendes force-pushed the async-windows branch from 2c3e9d0 to 49d335a Compare March 5, 2025 17:30

jprendes added 2 commits March 5, 2025 17:34

Add windows support on async

07f4df7

Add support for windows named pipes to the async codebase. This refactor the codes to use a transport abstraction instead of file descriptors. Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

fix CI failure

a687361

Fix failure with deny workflow in CI. This requires bumping cargo-deny-action, which introduces a breaking change in the syntax of deny.toml Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

jprendes force-pushed the async-windows branch from 49d335a to a687361 Compare March 5, 2025 17:35

jsturtevant reviewed Mar 5, 2025

View reviewed changes

src/asynchronous/server.rs Outdated Show resolved Hide resolved

src/asynchronous/transport/windows.rs Outdated Show resolved Hide resolved

jprendes added 2 commits March 5, 2025 23:38

Improve error message

3800f56

Improve the error message when a server is started with no listener specified Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

jsturtevant reviewed Mar 6, 2025

View reviewed changes

wllenyj approved these changes Mar 10, 2025

View reviewed changes

example/utils.rs Show resolved Hide resolved

jprendes mentioned this pull request Mar 14, 2025

make the async feature a no-op on non-unix platforms containerd/rust-extensions#380

Closed

jsturtevant approved these changes Mar 24, 2025

View reviewed changes

Tim-Zhang reviewed Mar 28, 2025

View reviewed changes

src/asynchronous/transport.rs Outdated Show resolved Hide resolved

Enable async examples in run_examples test

b362dd0

This commit enables on Windows the async examples in the run_examples test Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

jprendes force-pushed the async-windows branch from eeaeb11 to b362dd0 Compare March 28, 2025 15:58

move transport.rs to transport/mod.rs

5ea2e59

since transport.rs has submodules, move it from transport.rs to transport/mod.rs Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Tim-Zhang added the BREAKING CHANGE Introduces a breaking API change label Mar 28, 2025

Tim-Zhang linked an issue Mar 28, 2025 that may be closed by this pull request

Windows support #132

Closed

Tim-Zhang approved these changes Mar 31, 2025

View reviewed changes

Tim-Zhang merged commit 7ca128d into containerd:master Mar 31, 2025
12 checks passed

jprendes mentioned this pull request Apr 9, 2025

Bump crate versions #295

Open

KarstenB pushed a commit to KarstenB/ttrpc-rust that referenced this pull request May 1, 2025

Merge pull request containerd#290 from jprendes/async-windows

340e1cd

Add windows support on async

Add windows support on async #290

Add windows support on async #290

Uh oh!

Conversation

jprendes commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jprendes commented Mar 5, 2025

Uh oh!

jsturtevant left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Tim-Zhang commented Mar 6, 2025

Uh oh!

jsturtevant left a comment

Choose a reason for hiding this comment

Uh oh!

wllenyj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jprendes commented Mar 19, 2025

Uh oh!

jsturtevant commented Mar 24, 2025

Uh oh!

Uh oh!

Tim-Zhang commented Mar 28, 2025

Uh oh!

jprendes commented Mar 28, 2025

Uh oh!

jprendes commented Mar 28, 2025

Uh oh!

jprendes commented Mar 28, 2025

Uh oh!

Tim-Zhang commented Mar 28, 2025

Uh oh!

Tim-Zhang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jprendes commented Mar 4, 2025 •

edited

Loading