firezone / firezone

Enterprise-ready zero-trust access platform built on WireGuard®.
https://www.firezone.dev
Apache License 2.0
6.71k stars 279 forks source link

windows: use non-blocking mode for wintun #2887

Closed ReactorScram closed 7 months ago

ReactorScram commented 9 months ago

making an issue so I remember

https://github.com/firezone/firezone/blob/windows-headless-client/rust/connlib/tunnel/src/device_channel/tun/tun_windows.rs#L60-L108

ReactorScram commented 8 months ago

In-progress on the invalidate-dns branch

ReactorScram commented 8 months ago

This was working in commit e18fbf635 locally but I want to see if #2975 gets resolved first, just to be safe.

ReactorScram commented 8 months ago

I get about 33/10 down/up Mbps with the worker thread and blocking mode, so it's not the end of the world for performance. My Internet is supposed to be like 88/10 without the tunnel.

ReactorScram commented 8 months ago

I read a little of the RegisterWaitForSingleObject docs. Monday I need to:

Also the docs and SO indicate that RegisterWaitForSingleObject just uses a thread pool internally with threads that block waiting on objects. It might be better than using our own worker thread or using spawn_blocking, since other Windows stuff in our process maybe ends up sharing a wait thread somehow, but it's opaque. I could do a worker thread (since this is long-running, I think spawn_blocking wouldn't be right) with a timeout and a shutdown signal, basically how wintun's blocking recv does it, but idk.

There are many ways it could be done, none are great. After thinking about all this, I do want to ditch the worker thread, since wintun (the crate) internally does an infinite block on WaitForMultipleObjects, and I'd feel safer with a timeout there.

ReactorScram commented 8 months ago

I ended up taking a different approach, also removed the Arc, see PR

ReactorScram commented 8 months ago

Moving back to Todo, haven't worked on this for a few days and honestly I'd rather keep the blocking impl for a little longer

ReactorScram commented 7 months ago

Stopping this for now, we can come back to it at any time