async: add close_finished

[vbmithr]

I needed this to be able to detect connection cutoff in order to use Persistent_connection_kernel.Make and have persistent connections.

I ran into an issue while interacting with a gRPC service in a long polling fashion.

[vbmithr]

@anmonteiro ping?

[anmonteiro]

haven't had a chance to review this yet. It's going to be a few more days.

[anmonteiro]

I suspect this might not be needed but right now it doesn't hurt to have. I'm guessing if a program is running this branch, read_complete will always have been filled. Could be worth it to assert that.

[vbmithr]

Yeah, this async code is notoriously hard to follow, I don't recall very much now. All I know is this code works in prod. Agree for asserting things that we suppose true! :)

[anmonteiro]

Instead of introducing a new API, could we make this part of the val shutdown: _ t -> unit Deferred.t process?

[vbmithr]

To be used as a functor for Persistent_connection_kernel.Make, it should follow this interface:

module type Closable = sig
  (** a connection type *)
  type t

  (** [close t] closes the connection. The returned deferred becomes determined once any
      resources needed to maintain the connection have been released. *)
  val close : t -> unit Deferred.t

  (** [is_closed t] returns true if [close] has ever been called (even if the returned
      deferred has not yet been fulfilled).

      Note that some modules implementing [Closable] may call close internally upon
      noticing that the connection was closed by the other side. The interface of such a
      module ought to say that this is the case. *)
  val is_closed : t -> bool

  (** [close_finished t] becomes determined at the same time as the result of the first
      call to [close]. [close_finished] differs from [close] in that it does not have the
      side effect of initiating a close. *)
  val close_finished : t -> unit Deferred.t
end

[vbmithr]

Actually, since there is a parameter it cannot be used directly as an argument to Persistent_connection_kernel.Make. I used this patch in an internal fork to ocaml-h2 that I could maybe upstream at some point.

[vbmithr]

But as the doc says, close and close_finished have not the same semantics.
Also, would not be better to call shutdown -> close since the socket is closed in the process?

[anmonteiro]

• leaving this here for my future reference: https://github.com/janestreet/async_kernel/blob/9f747452c88fe716cda4bd6f29ef0f3cb1ff3a39/persistent_connection_kernel/src/persistent_connection_kernel_intf.ml#L15

would not be better to call shutdown -> close since the socket is closed in the process?

works for me, would you like to propose a separate PR for that?

[anmonteiro]

could you allow me to push to your branch (here are some instructions)?

I have a few cosmetic fixes I'd like to push.

[vbmithr]

I could not find how to give you access to the PR, gave you write access to the repo instead.

[vbmithr]

@anmonteiro Could anything be done for this PR? I need this patch to use other of your code, notably httpun.