vbmithr / ocaml-websocket

Websocket library for OCaml
ISC License
161 stars 44 forks source link

Don't close socket on Lwt.Canceled. #129

Open NightBlues opened 1 year ago

NightBlues commented 1 year ago

Hi! I want to write recv code with timeouts like this (just synthetic example):

 open Lwt

 ...
 let myrecv ~timeout conn =
   let action_th = Websocket_lwt_unix.read conn >>= fun x -> return (Some x) in
   let timeout_th = Lwt_unix.sleep timeout >>= fun () -> return None in
   Lwt.pick [ timeout_th ; action_th ]

 ...
 myrecv ~timeout:3.0 >>= function
 | None -> (* TODO: do something else and try again *)
 | Some x -> ...

Could you please consider this PR

@vbmithr @paurkedal

paurkedal commented 1 year ago

I am not sure this would be a reliable way of implementing a timeout. If the cancel happens in between the individual read operations in websocket.read_frame, will we not end up with an out of sync stream on the input channel, from which it will be hard to recover?

NightBlues commented 1 year ago

Not sure what does "out of sync stream" mean - you mean that we can loose some frames? For now if Lwt.cancel happens - we have invalid connection - with working fine write socket oc and closed read socket ic (and when we try to read from closed socket we get End_of_file exception)

paurkedal commented 1 year ago

If you look at core/websocket.ml starting at line 278, the invocation of read_uint16 or read_uint64 is followed by two read_exactly. If the cancel happens before either of the two read_exactly, there will be bytes left on ic from that frame, which need to be discarded before processing the next frame.