Open so-schen opened 7 months ago
@mmastrac, by reading the impl, it seems to me the read frame is not Cancel safe. Am I correct?
I suspect if you are using automatic pong and automatic close, those methods are not cancellation-safe. We probably need to have a full poll-read/poll-write set of interfaces to ensure that.
FragmentCollector::read_frame()
which use async fn parse_frame_header
does not remember the read pointer, so if it is canceled in the middle, the data read from tcp stream will be forgotten in next call, because re-enter the read_frame
, we start from read from index 0
I am not familiar with websocket wired-protocol, I wondering if this is potential risk
FragmentCollector::read_frame()
which useasync fn parse_frame_header
does not remember the read pointer, so if it is canceled in the middle, the data read from tcp stream will be forgotten in next call, because re-enter theread_frame
, we start from read from index0
I am not familiar with websocket wired-protocol, I wondering if this is potential risk
Definitely FragmentCollector::read_frame()
is not cancel safe (regardless if automatic pong/close is set or not). If the future is cancelled the framing logic might be interrupted in the middle and, thus, next read will produce a protocol error.
It's been in use in Deno for a few months now, I think and it's been stable. I'm not sure if we'll tweak the API in the future to make it a little more ergonomic, but the feature definitely works.