From 1634b597f4f93aa871f54ea74cbff0c96d111f36 Mon Sep 17 00:00:00 2001 From: Patrick Ferris Date: Fri, 10 Jul 2026 11:20:13 +0100 Subject: [PATCH] Improved signal handling This is better signal handling for SIGINT. We need to make sure child processes have the default interrupt handling, but that the shell has whatever "special" SIGINT handling it needs. Before this change, after running a program like `ls -la`, a ctrl-c would exit the shell! --- src/lib/eunix.ml | 1 + src/lib/eval.ml | 19 +++++++++++++++++++ src/lib/job.ml | 9 +++++++-- src/lib/types.ml | 6 ++++++ src/lib_posix/exec.ml | 1 - test/fd_table.t | 18 +++++++----------- 6 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/lib/eunix.ml b/src/lib/eunix.ml index 2dc7629..e911f43 100644 --- a/src/lib/eunix.ml +++ b/src/lib/eunix.ml @@ -253,4 +253,5 @@ module Signals = struct | m -> Fmt.invalid_arg "Signal %s not supported or recognised." m let raise v = Unix.kill (Unix.getpid ()) (to_int v) + let send ~pid v = Unix.kill pid (to_int v) end diff --git a/src/lib/eval.ml b/src/lib/eval.ml index 0ff03bd..7eb328a 100644 --- a/src/lib/eval.ml +++ b/src/lib/eval.ml @@ -429,6 +429,21 @@ module Make (S : Types.State) (E : Types.Exec) = struct let mode = if async then Types.Switched ctx.async_switch else Types.Switched sw in + (* Before we execute the first program, we need to set the default sigint behaviour...*) + let job = + match J.get_sigint job with + | None -> + let old_behaviour = Sys.signal Sys.sigint Sys.Signal_default in + J.set_sigint old_behaviour job + | Some _ -> job + in + let job = + match J.get_sigint job with + | None -> + let old_behaviour = Sys.signal Sys.sigint Sys.Signal_default in + J.set_sigint old_behaviour job + | Some _ -> job + in let ctx, process = let hash, prog = Eunix.resolve_program ~path:(S.path ctx.state) ctx.hash executable @@ -980,6 +995,8 @@ module Make (S : Types.State) (E : Types.Exec) = struct (fun m e -> Fd_table.merge m (Exit.value e).fd_table) Fd_table.empty exit_ctxs in + (* Restore the sigint handler *) + Option.iter (fun v -> Sys.set_signal Sys.sigint v) (J.get_sigint job); (* Guranteed to have a head as job size > 0 *) let exit_ctx = List.hd exit_ctxs in exit_ctx >|= fun ctx -> @@ -992,6 +1009,8 @@ module Make (S : Types.State) (E : Types.Exec) = struct } end else begin + (* Restore the sigint handler *) + Option.iter (Sys.set_signal Sys.sigint) (J.get_sigint job); (* We must allow async jobs to be reaped, J.await_exit does this for us. *) J.reap job; let last_process = diff --git a/src/lib/job.ml b/src/lib/job.ml index d003ad7..21fd9a6 100644 --- a/src/lib/job.ml +++ b/src/lib/job.ml @@ -4,6 +4,9 @@ module Make (E : Types.Exec) = struct type 'a t = { reap : unit Eio.Promise.t * unit Eio.Promise.u; id : int; + (* If [Some b] then we have set sigint to stop the job, and [b] is + the previous signal behaviour that we can restore later. *) + sigint : Sys.signal_behavior option; (* Process list is in reverse order *) processes : [ `Process of 'a * process @@ -37,15 +40,17 @@ module Make (E : Types.Exec) = struct let get_id t = t.id let set_id new_id t = { t with id = new_id } + let get_sigint t = t.sigint + let set_sigint sigint t = { t with sigint = Some sigint } let get_reaper t = t.reap let reap t = let _, r = t.reap in try Eio.Promise.resolve r () with Invalid_argument _ -> () - let make id processes = + let make ?sigint id processes = let reap = Eio.Promise.create ~label:("reap-" ^ string_of_int id) () in - { id; processes; reap } + { id; sigint; processes; reap } let add_process ctx proc t = { t with processes = List.cons (`Process (ctx, proc)) t.processes } diff --git a/src/lib/types.ml b/src/lib/types.ml index 77f31eb..fe23772 100644 --- a/src/lib/types.ml +++ b/src/lib/types.ml @@ -140,6 +140,7 @@ module type Job = sig val reap : _ t -> unit val make : + ?sigint:Sys.signal_behavior -> int -> [ `Built_in of 'a Exit.t Eio.Promise.or_exn | `Noop of 'a Exit.t @@ -159,6 +160,11 @@ module type Job = sig val set_id : int -> 'a t -> 'a t (** Set the ID of the job. *) + val set_sigint : Sys.signal_behavior -> 'a t -> 'a t + + val get_sigint : _ t -> Sys.signal_behavior option + (** Get the previous signal behaviour for sigint. *) + val add_process : 'a -> process -> 'a t -> 'a t val add_built_in : 'a Exit.t Eio.Promise.or_exn -> 'a t -> 'a t val add_error : int -> 'a t -> 'a t diff --git a/src/lib_posix/exec.ml b/src/lib_posix/exec.ml index ecd50e5..c33e9f9 100644 --- a/src/lib_posix/exec.ml +++ b/src/lib_posix/exec.ml @@ -265,7 +265,6 @@ let pp_redirections ppf (i, fd, _) = Fmt.pf ppf "(%i,%a)" i Eio_unix.Fd.pp fd let run ~mode ?delay_reap _ ?stdin ?stdout ?stderr ?(fds = []) ?(fork_actions = []) ~pgid ~cwd ~pipe ?env ?executable args = - Sys.set_signal Sys.sigint Sys.Signal_default; with_close_list @@ fun to_close -> let check_fd n = function | Merry.Types.Redirect (_, m, _, _) -> Int.equal n m diff --git a/test/fd_table.t b/test/fd_table.t index df7e192..cb515b8 100644 --- a/test/fd_table.t +++ b/test/fd_table.t @@ -1,14 +1,10 @@ Keeping track of the file descriptors in-memory (not just the process itself). - $ cat > test.sh << EOF - > exec 4>&1 - > msh-info - > EOF +This test is disabled as the output is not reproducible. - $ msh test.sh - msh-info - {quotes: none, - fd-table: - [4 -> FD-1, 455 -> Path-pipe:[818672], 456 -> Path-pipe:[818672]], - argv: [test.sh], - functions: []} +$ cat > test.sh << EOF +> exec 4>&1 +> msh-info +> EOF + +$ msh test.sh -- 2.51.2