From e3143625e2a7d3c89adbda9b5b442c6d434b038a Mon Sep 17 00:00:00 2001 From: Gabe Levi Date: Tue, 3 Apr 2018 07:13:25 -0700 Subject: [PATCH] Switch to using nonblocking fds with lwt Summary: This is the result of a week long investigation into Flow occasionally hanging. Here's what you should know: * A read/write of a blocking fd will block until the fd is readable/writable * A read/write of a nonblocking fd will return -1 immediately if the fd is not readable/writable * Lwt super extra hates running blocking system calls. It will create system threads and run the blocking system call on them * There's a bug in Lwt in scheduling the system threads: https://github.com/ocsigen/lwt/issues/569 From our point of view, there's very little difference in using blocking or non-blocking fds. We had just been using blocking fds since I couldn't really tell if one was better than the other, However, using non-blocking keeps us from needing system threads and works around the bug, so let's use them! Reviewed By: samwgoldman Differential Revision: D7464034 fbshipit-source-id: e0ba602381a8bef7dd374ee1cd5fb0fdef9ad7d9 --- hack/dfind/dfindLibLwt.ml | 4 ++-- hack/procs/workerControllerLwt.ml | 4 ++-- src/monitor/flowServerMonitor.ml | 8 +++++--- src/monitor/flowServerMonitorServer.ml | 4 ++-- src/monitor/logger/flowServerMonitorLogger.ml | 2 +- src/monitor/monitorRPC.ml | 3 ++- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/hack/dfind/dfindLibLwt.ml b/hack/dfind/dfindLibLwt.ml index d72f70f8335..4d2f5a6d86a 100644 --- a/hack/dfind/dfindLibLwt.ml +++ b/hack/dfind/dfindLibLwt.ml @@ -17,8 +17,8 @@ struct let return = Lwt.return let (>>=) = Lwt.(>>=) - let descr_of_in_channel ic = Lwt_unix.of_unix_file_descr (Daemon.descr_of_in_channel ic) - let descr_of_out_channel oc = Lwt_unix.of_unix_file_descr (Daemon.descr_of_out_channel oc) + let descr_of_in_channel ic = Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true (Daemon.descr_of_in_channel ic) + let descr_of_out_channel oc = Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true (Daemon.descr_of_out_channel oc) let to_fd_with_preamble ?timeout ?flags fd v = if timeout <> None diff --git a/hack/procs/workerControllerLwt.ml b/hack/procs/workerControllerLwt.ml index 9afeb5b3f81..cb2059dfa29 100644 --- a/hack/procs/workerControllerLwt.ml +++ b/hack/procs/workerControllerLwt.ml @@ -40,8 +40,8 @@ let call w (type a) (type b) (f : a -> b) (x : a) : b Lwt.t = let infd = Daemon.descr_of_in_channel inc in let outfd = Daemon.descr_of_out_channel outc in - let infd_lwt = Lwt_unix.of_unix_file_descr ~blocking:true infd in - let outfd_lwt = Lwt_unix.of_unix_file_descr ~blocking:true outfd in + let infd_lwt = Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true infd in + let outfd_lwt = Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true outfd in let request = wrap_request w f x in diff --git a/src/monitor/flowServerMonitor.ml b/src/monitor/flowServerMonitor.ml index f03a8bf1c6d..942dcdcb83e 100644 --- a/src/monitor/flowServerMonitor.ml +++ b/src/monitor/flowServerMonitor.ml @@ -126,7 +126,7 @@ let internal_start ~is_daemon ?waiting_fd monitor_options = let handle_waiting_start_command = match waiting_fd with | None -> Lwt.return_unit | Some fd -> - let fd = Lwt_unix.of_unix_file_descr ~blocking:true fd in + let fd = Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true fd in handle_waiting_start_command fd in @@ -139,11 +139,13 @@ let internal_start ~is_daemon ?waiting_fd monitor_options = (* We can start up the socket acceptor even before the server starts *) Lwt.async (fun () -> SocketAcceptor.run - (Lwt_unix.of_unix_file_descr ~blocking:true monitor_socket_fd) + (Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true monitor_socket_fd) monitor_options.FlowServerMonitorOptions.autostop ); Lwt.async (fun () -> - SocketAcceptor.run_legacy (Lwt_unix.of_unix_file_descr ~blocking:true legacy_socket_fd) + SocketAcceptor.run_legacy ( + Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true legacy_socket_fd + ) ); (* Wait forever! Mwhahahahahaha *) diff --git a/src/monitor/flowServerMonitorServer.ml b/src/monitor/flowServerMonitorServer.ml index 4ec3f8c1220..b3ba876c8ac 100644 --- a/src/monitor/flowServerMonitorServer.ml +++ b/src/monitor/flowServerMonitorServer.ml @@ -165,11 +165,11 @@ end = struct let in_fd = ic |> Daemon.descr_of_in_channel - |> Lwt_unix.of_unix_file_descr ~blocking:true in + |> Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true in let out_fd = oc |> Daemon.descr_of_out_channel - |> Lwt_unix.of_unix_file_descr ~blocking:true in + |> Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true in let close_if_open fd = try Lwt_unix.close fd diff --git a/src/monitor/logger/flowServerMonitorLogger.ml b/src/monitor/logger/flowServerMonitorLogger.ml index a99562ac786..f825faf1ddf 100644 --- a/src/monitor/logger/flowServerMonitorLogger.ml +++ b/src/monitor/logger/flowServerMonitorLogger.ml @@ -75,7 +75,7 @@ let init_logger log_fd = let template = "$(date).$(milliseconds) [$(level)] $(message)" in - let log_fd = Option.map log_fd ~f:Lwt_unix.of_unix_file_descr in + let log_fd = Option.map log_fd ~f:(Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true) in let fds = Lwt_unix.stderr :: (Option.value_map log_fd ~default:[] ~f:(fun fd -> [fd])) in Lwt.async (fun () -> WriteLoop.run fds); diff --git a/src/monitor/monitorRPC.ml b/src/monitor/monitorRPC.ml index 2751eee9fcd..adb6abab690 100644 --- a/src/monitor/monitorRPC.ml +++ b/src/monitor/monitorRPC.ml @@ -31,7 +31,8 @@ let with_outfd ~on_disabled ~f = with_channel snd ~on_disabled ~f (* The main server process will initialize this with the channels to the monitor process *) let init ~channels:(ic,oc) = - let infd = Lwt_unix.of_unix_file_descr (Daemon.descr_of_in_channel ic) in + let infd = + Lwt_unix.of_unix_file_descr ~blocking:false ~set_flags:true (Daemon.descr_of_in_channel ic) in let outfd = Daemon.descr_of_out_channel oc in state := Initialized {infd; outfd}