Lwt-unix: Fix file descriptor leak with Lwt_unix.shutdown

[francoisthire]

I stumbled across a case where the syscall shutdown of my application failed with ENOTCONN and the syscall to close did not occur resulting into a file descriptor leak.

By looking at the potential culprit, I found the shutdown function of this library.

The patch is inspired to what is done in lwt_io:

let close_socket fd =
  Lwt.finalize
    (fun () ->
       Lwt.catch
         (fun () ->
            Lwt_unix.shutdown fd Unix.SHUTDOWN_ALL;
            Lwt.return_unit)
         (function
           (* Occurs if the peer closes the connection first. *)
           | Unix.Unix_error (Unix.ENOTCONN, _, _) -> Lwt.return_unit
           | exn -> Lwt.reraise exn))
    (fun () ->
       Lwt_unix.close fd)

(for reference, another similar MR: ocsigen/lwt_ssl#5)

[anmonteiro]

it makes sense, but can you reproduce the fix in lwt_io faithfully? we shouldn't be using try with Lwt

[francoisthire]

I have force-pushed the commit mimicking the lwt_io function. I believe that for this particular case it does not change anything since the shutdown function is not in lwt though. So the try ... with will work as expected.

[anmonteiro]

I believe that for this particular case it does not change anything since the shutdown function is not in lwt though. So the try ... with will work as expected.

I agree but it required going looking for the implementation to know. That shouldn’t be required.

[francoisthire]

@anmonteiro What is the next step to unblock this? Let me know if I can do anything to help!

[yawaramin]

it required going looking for the implementation to know. That shouldn’t be required.

@anmonteiro the Lwt_unix.shutdown function return type is unit, so it can't return a rejected promise; therefore we can use regular try...with.