8000 Connection closed by server does not call error handler in client · Issue #177 · anmonteiro/ocaml-h2 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Connection closed by server does not call error handler in client #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community. 8000

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ansiwen opened this issue Jul 27, 2022 · 1 comment · Fixed by #196
Closed

Connection closed by server does not call error handler in client #177

ansiwen opened this issue Jul 27, 2022 · 1 comment · Fixed by #196

Comments

@ansiwen
Copy link
ansiwen commented Jul 27, 2022 •
edited
Loading

If the server closes a connection, there seems to be no way for the client to notice that (besides keepalive timeouts). I would expect that the error handler for the connection-level errors is called, or at least the connection changes into closed-state and all pending promises on that connection being canceled, which all apparently doesn't happen.

Here the code that I used to verify the behaviour outside of my application:

open Lwt.Infix

let error_to_string e =
  match e with
  | `Exn e -> Printexc.to_string e
  | `Malformed_response s -> Format.sprintf "malformed response: %s" s
  | `Invalid_response_body_length r ->
      Format.asprintf "invalid response body length: %a" H2.Response.pp_hum r
  | `Protocol_error (c, s) ->
      Format.asprintf "protocol error %a: %s" H2.Error_code.pp_hum c s

let create_connection ~host ~port =
  print_endline "create_connection";
  Lwt_unix.getaddrinfo host (string_of_int port) [] >>= fun addresses ->
  let socket = Lwt_unix.socket Unix.PF_INET Unix.SOCK_STREAM 0 in
  let error_handler e =
    let msg = error_to_string e in
    print_endline "connection error";
    print_endline msg
  in
  match addresses with
  | { Unix.ai_addr; _ } :: _ ->
      Lwt_unix.connect socket ai_addr >>= fun () ->
      print_endline "TCP connected";
      let conn = H2_lwt_unix.Client.create_connection ~error_handler socket in
      print_endline "H2 connected";
      conn
  | _ -> assert false

let () =
  let host, port =
    try (Sys.argv.(1), int_of_string Sys.argv.(2))
    with Invalid_argument _ ->
      Printf.eprintf "Usage: %s ETCD_HOST ETCD_PORT\n" Sys.argv.(0);
      exit 1
  in
  let main =
    create_connection ~host ~port >>= fun connection ->
    let rec loop () =
      let promise, resolver = Lwt.task () in
      H2_lwt_unix.Client.ping connection (fun () ->
          Lwt.wakeup_later resolver ());
      print_endline "ping";
      promise >>= fun () ->
      print_endline "pong";
      Lwt_unix.sleep 1.0 >>= loop
    in
    loop ()
  in
  Lwt_main.run main
@dinosaure
Copy link

I can confirm that this issue exists with paf-le-chien which is a file-descriptor leak (see this issue on our opam-mirror unikernel: https://git.robur.io/robur/opam-mirror/issues/5).

I first thought about the need for a Unix.shutdown_send which could close the connection and signal the server to finally emit a final packet so that the reading loop would also close properly, but I'm not too sure anymore - other limitations within MirageOS don't allow us to experiment with this solution yet.

@anmonteiro anmonteiro mentioned this issue Oct 21, 2022
Merged
@anmonteiro anmonteiro mentioned this issue Mar 17, 2023
Merged
anmonteiro added a commit to anmonteiro/opam-repository that referenced this issue Mar 17, 2023
@anmonteiro
[new release] hpack, h2, h2-mirage, h2-lwt, h2-lwt-unix, h2-eio and h…
e2acf10 
…2-async (0.10.0)

CHANGES:

- hpack: fix a case where hpack would raise an array out of bounds exception
  ([anmonteiro/ocaml-h2#183](anmonteiro/ocaml-h2#183))
  ([@jonathanjameswatson](https://github.com/jonathanjameswatson))
- h2: (client) handle multiple RST_STREAM frames
  ([anmonteiro/ocaml-h2#184](anmonteiro/ocaml-h2#184))
  ([@jonathanjameswatson](https://github.com/jonathanjameswatson))
- h2: (client) Fix a race condition with `~flush_headers_immediately:false` and
  empty request bodies
  ([anmonteiro/ocaml-h2#186](anmonteiro/ocaml-h2#186))
- h2: Make `H2.Reqd.error_code` part of the public interface
  ([anmonteiro/ocaml-h2#188](anmonteiro/ocaml-h2#188))
- h2: Add `~request_method` argument to `H2.Method.body_length`
  ([anmonteiro/ocaml-h2#190](anmonteiro/ocaml-h2#190))
  ([@jonathanjameswatson](https://github.com/jonathanjameswatson))
- h2: Don't send any frames on a stream after an `RST_STREAM` frame
  ([anmonteiro/ocaml-h2#187](anmonteiro/ocaml-h2#187),
  [anmonteiro/ocaml-h2#194](anmonteiro/ocaml-h2#194))
- h2: call error handler on the client if the remote peer closes the
  commmunication channel
  ([anmonteiro/ocaml-h2#177](anmonteiro/ocaml-h2#177),
  [anmonteiro/ocaml-h2#196](anmonteiro/ocaml-h2#194))
- h2: when reprioritizing a stream, respect its new priority (accounts for
  inferred default priority when a dependent stream is not in the tree
  ([RFC7540§5.3.1](https://www.rfc-editor.org/rfc/rfc7540.html#section-5.3.1)))
  ([anmonteiro/ocaml-h2#200](anmonteiro/ocaml-h2#200))
- h2: don't remove parent streams from the scheduler if they have children
  ([anmonteiro/ocaml-h2#201](anmonteiro/ocaml-h2#201))
- h2: don't schedule streams as dependencies of others marked for removal
  ([anmonteiro/ocaml-h2#205](anmonteiro/ocaml-h2#205))
- h2: revise scheduling algorithm to avoid starvation
  ([anmonteiro/ocaml-h2#199](anmonteiro/ocaml-h2#199),
  [anmonteiro/ocaml-h2#204](anmonteiro/ocaml-h2#204), reported in
  [anmonteiro/ocaml-h2#162](anmonteiro/ocaml-h2#162), thanks
  [@quernd](https://github.com/quernd))
- h2-eio: adapt to the next gluten-eio version
  ([anmonteiro/ocaml-h2#210](anmonteiro/ocaml-h2#210))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[h2.client]: call error handler when the server closes the connection anmonteiro/ocaml-h2
2 participants
@dinosaure @ansiwen
0