refactor(rpc): create lifecycle server eagerly

bin/scheduler_setup.ml

@@ -19,12 +19,18 @@ let go_without_rpc_server ~(common : Common.t) ~config:dune_config f =
 ;;
 
 let await_action_runner common =
-  match Common.action_runner common with
-  | None -> Fiber.return ()
-  | Some action_runner ->
-    let open Fiber.O in
+  let open Fiber.O in
+  if Common.action_runner_requested common
+  then
+    (* [Common.action_runner] spawns a worker that immediately connects back to
+       Dune over RPC. With eager socket creation, the socket can be connectable
+       before the accept loop is running, so start the server before spawning
+       the worker. *)
     let* () = Dune_rpc_impl.Server.ensure_ready () in
-    Dune_engine.Action_runner.ensure_ready action_runner
+    match Common.action_runner common with
+    | None -> Fiber.return ()
+    | Some action_runner -> Dune_engine.Action_runner.ensure_ready action_runner
+  else Fiber.return ()
 ;;
 
 let go_with_rpc_server ~common ~config f =

src/dune_rpc_impl/server.ml

@@ -108,16 +108,6 @@ let () =
     | Some server -> to_dyn server)
 ;;
 
-let ready (t : t) =
-  Rpc.Server.Lifecycle.ready t.server.lifecycle
-  >>= function
-  | Ok () -> Fiber.return ()
-  | Error exn ->
-    Dune_util.Report_error.report exn;
-    Scheduler.shutdown `Failure;
-    raise Dune_util.Report_error.Already_reported
-;;
-
 let stop (t : t) =
   Fiber.fork_and_join_unit
     (fun () -> Action_runner.Rpc_server.stop t.server.action_runner)
@@ -442,6 +432,7 @@ let create ~registry ~root ~build watch_mode =
     in
     let action_runner = Action_runner.Rpc_server.create () in
     let handler = Rpc.Server.make (handler t action_runner) in
+    let server = Lazy.force server in
     let lifecycle = Rpc.Server.Lifecycle.create ~handler ~root ~where ~registry ~server in
     action_runner, lifecycle
   in
@@ -518,8 +509,7 @@ module Background = struct
     | `Running -> Fiber.return ()
     | `Awaiting_start ->
       t.state <- `Running;
-      let* () = Fiber.Pool.task pool ~f:(fun () -> run server) in
-      ready server
+      Fiber.Pool.task pool ~f:(fun () -> run server)
   ;;
 end
 

src/rpc/server.ml

@@ -889,56 +889,39 @@ module Lifecycle = struct
   type nonrec t =
     { handler : t
     ; registry : Rpc_registry.t
-    ; server : Csexp_rpc.Server.t Lazy.t
-    ; startup_ivar : (Csexp_rpc.Server.t, Exn_with_backtrace.t) result Fiber.Ivar.t
+    ; server : Csexp_rpc.Server.t
     }
 
   let create ~handler ~root ~where ~registry ~server =
     let registry = Rpc_registry.create ~root ~where registry in
-    { handler; registry; server; startup_ivar = Fiber.Ivar.create () }
+    Rpc_registry.register registry;
+    { handler; registry; server }
   ;;
 
   let print_uncaught_rpc_error exn =
     Console.print [ Pp.text "Uncaught RPC Error"; Exn_with_backtrace.pp exn ]
   ;;
 
   let run t =
-    let with_print_errors f () =
-      Fiber.with_error_handler f ~on_error:(fun exn ->
-        print_uncaught_rpc_error exn;
-        Exn_with_backtrace.reraise exn)
-    in
     let run () =
-      match Exn_with_backtrace.try_with (fun () -> Lazy.force t.server) with
-      | Error exn ->
-        Dune_trace.emit Rpc (fun () -> Dune_trace.Event.Rpc.startup_failure exn);
-        print_uncaught_rpc_error exn;
-        Fiber.Ivar.fill t.startup_ivar (Error exn)
-      | Ok server ->
-        let* () = Fiber.Ivar.fill t.startup_ivar (Ok server) in
-        let* sessions = Csexp_rpc.Server.serve server in
-        let () = Rpc_registry.register t.registry in
-        serve sessions t.handler
+      let* sessions = Csexp_rpc.Server.serve t.server in
+      serve sessions t.handler
     in
-    with_print_errors run
-    |> Fiber.finalize ~finally:(fun () ->
-      Rpc_registry.cleanup t.registry;
-      Fiber.return ())
-  ;;
-
-  let ready t =
-    Fiber.Ivar.read t.startup_ivar
-    >>= function
-    | Ok _ -> Fiber.return (Ok ())
-    | Error _ as error -> Fiber.return error
+    Fiber.finalize
+      (fun () ->
+         Fiber.with_error_handler run ~on_error:(fun exn ->
+           print_uncaught_rpc_error exn;
+           Exn_with_backtrace.reraise exn))
+      ~finally:(fun () ->
+        Rpc_registry.cleanup t.registry;
+        Fiber.return ())
   ;;
 
   let stop t =
-    Fiber.of_thunk (fun () ->
-      (* CR-soon rgrinberg: this is weird. we shouldn't be ignoring [stop] like this *)
-      match Fiber.Ivar.peek t.startup_ivar with
-      | None -> Fiber.return ()
-      | Some (Error _) -> Fiber.return ()
-      | Some (Ok server) -> Csexp_rpc.Server.stop server)
+    Fiber.finalize
+      (fun () -> Csexp_rpc.Server.stop t.server)
+      ~finally:(fun () ->
+        Rpc_registry.cleanup t.registry;
+        Fiber.return ())
   ;;
 end

src/rpc/server.mli

@@ -163,11 +163,10 @@ module Lifecycle : sig
       -> root:string
       -> where:Where.t
       -> registry:[ `Add | `Skip ]
-      -> server:Csexp_rpc.Server.t Lazy.t
+      -> server:Csexp_rpc.Server.t
       -> t
 
     val run : t -> unit Fiber.t
-    val ready : t -> (unit, Exn_with_backtrace.t) result Fiber.t
     val stop : t -> unit Fiber.t
   end
   with type handler := t

test/blackbox-tests/test-cases/cram/custom-build-dir.t

@@ -18,9 +18,12 @@ path
   File "foo.t", line 1, characters 0-0:
   --- foo.t
   +++ foo.t.corrected
-  @@ -6,3 +6,10 @@
+  @@ -4,5 +4,12 @@
+     > else
+     >   echo ".rpc missing"
      > fi
-     .rpc missing
+  -  .rpc missing
+  +  .rpc exists
      $ env -u DUNE_RPC -u DUNE_BUILD_DIR dune runtest
   +  File "bar.t", line 1, characters 0-0:
   +  --- bar.t
@@ -40,7 +43,7 @@ path
     > else
     >   echo ".rpc missing"
     > fi
-    .rpc missing
+    .rpc exists
     $ env -u DUNE_RPC -u DUNE_BUILD_DIR dune runtest
     File "bar.t", line 1, characters 0-0:
     --- bar.t

test/blackbox-tests/test-cases/watching/rpc-bind-failure.t

@@ -16,19 +16,13 @@ _build/.rpc/dune fails during startup.
 
   $ wait_for_dune_exit_with_timeout
 
-  $ grep "Uncaught RPC Error" "$OUTPUT"
-  Uncaught RPC Error
+The important behavior is that the process exits promptly and reports the bind
+failure.
 
   $ grep '^Error: bind(): Not a directory$' "$OUTPUT"
   Error: bind(): Not a directory
 
   $ grep '^exit 1$' "$OUTPUT"
   exit 1
 
-  $ dune trace cat | jq -c '
-  >    select(.cat == "rpc" and .name == "startup-failure")
-  > | { name, error: .args.error.exn }
-  > '
-  {"name":"startup-failure","error":"Unix.Unix_error(Unix.ENOTDIR, \"bind\", \"\")"}
-
   $ ! with_timeout_quiet dune rpc ping >/dev/null 2>&1