search

snapview / tokio-tungstenite

Future-based Tungstenite for Tokio. Lightweight stream-based WebSocket implementation
MIT License
1.86k stars 234 forks source link

WakerProxy may be unnecessary #163

Open quininer opened 3 years ago

quininer commented 3 years ago

I found it from deno's discussion.

tokio-tungstenite uses the trick of WakerProxy to solve the wake-up problem of two-way IO. but this is not necessary in tokio, the related problem is reported as https://github.com/tokio-rs/tls/issues/40 .

In short, the latest versions of tokio::net::TcpStream andtokio-rustls are not affected by this issue.

Given that this trick may cause lot spurious wakeups, we may be possible to provide an opt-out feature gate?

strohel commented 3 years ago

Thanks for a heads-up, @quininer. I'm not expert on that part of the code, but it indeed looks like we can now simplify thanks to upstream bugfixes. CC @application-developer-DA @jxs @agalakhov.

jxs commented 3 years ago

hi @quininer, tokio-tungstenite needs a compat layer similar to the one used on tokio-native-tls as tungstenite uses Read and Write traits directly. It started to use the same trick of tokio-native-tls but was since upgraded to use the WakerProxy trick on https://github.com/snapview/tokio-tungstenite/commit/6e566eced79d115d151df9d2efdc75482bb90b54

I see two possibilities: 1 - revert to use the same technique as tokio-native-tls, is this what you are suggesting too right @quininer ? (has the unsafe disadvantage but is simple to implement) 2 - decouple base tungstenite from IO (similar to how quiche was conceived, also see the SansIO manifesto) and have IO implementations depend on it (considerable time needed to try to implement, not 100% sure if feasible)

I would suggest going for 1 now, and consider 2 in the mid term cc @sdroege

quininer commented 3 years ago

This sounds great, and I am looking forward to 2. It would be great if there is a websocket library that works under no_std.