search

n0-computer / quic-rpc

A streaming rpc system based on quic
Other
89 stars 12 forks source link

fix: rpc client concurrently waits for requests and new connections #62

Closed divagant-martian closed 6 months ago

divagant-martian commented 6 months ago

Replaces #55

Fixes handling a stale connection with an active request, which previously behaved as quic-rpc not realizing the server has left and producing the somewhat obscure error Open(LocallyClosed).

Additionally, it fixes quic-rpc hanging when the server is not connected. This was caused by handling obtaining a connection to the server and reading the channel for requests in a sequential manner. To solve this we use instead a tokio::select to concurrently wait for requests and new connections.

Note that part of the code complexity introduced is due to flume's receiving future not being cancel safe.

Fix includes a test, and I have tested it in iroh and the problematic behaviour is no longer there.

divagant-martian commented 6 months ago

I added a link in the code to a flume issue. They are aware of their futures not being cancel safe, but it seems to be they are fine with it. Linking here for completeness.

I have not checked how different the code would be with a futures::future::select, but yeah, it would no allow with the cancel safety issue. If you still prefer the alternative select I can give it a go at checking what it looks like