about summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorbptato <nincsnevem662@gmail.com>2023-06-25 22:53:57 +0200
committerbptato <nincsnevem662@gmail.com>2023-06-26 00:46:53 +0200
commitd0c4570fffec4c4d9db909f59b7f70c89e012acf (patch)
tree2657c9686c2854f5cc2a5486a80b88a788b2266d
parent0524bed395cfeb467812854c55673c8dc87a1bde (diff)
downloadchawan-d0c4570fffec4c4d9db909f59b7f70c89e012acf.tar.gz
Fix crash in openEditor if SIGINT was delivered
nvi for example sets ISIG and traps SIGINT.
Without this patch, this would propagate to all processes in the same
process group and kill them. (It still does, but we set a signalHandler
to ignore that.)

Still not perfect, because for some reason we receive WIFSIGNALED even
if the signal did not actually kill the editor. For now, we just treat
this as a success.
-rw-r--r--res/config.toml1
-rw-r--r--src/config/config.nim1
-rw-r--r--src/display/client.nim1
-rw-r--r--src/ips/editor.nim14
-rw-r--r--src/ips/forkserver.nim15
5 files changed, 29 insertions, 3 deletions
diff --git a/res/config.toml b/res/config.toml
index ae1d5984..edd5e255 100644
--- a/res/config.toml
+++ b/res/config.toml
@@ -14,6 +14,7 @@ display-charset = "auto"
 [external]
 tmpdir = "/tmp/cha"
 editor = "vi %s +%d"
+editor-accept-sigint = false
 
 [network]
 max-redirect = 10
diff --git a/src/config/config.nim b/src/config/config.nim
index 533ac93f..112f4398 100644
--- a/src/config/config.nim
+++ b/src/config/config.nim
@@ -75,6 +75,7 @@ type
   ExternalConfig = object
     tmpdir*: string
     editor*: string
+    editor_accept_sigint*: bool
 
   NetworkConfig = object
     max_redirect*: int32
diff --git a/src/display/client.nim b/src/display/client.nim
index 81d400b1..49dae2ea 100644
--- a/src/display/client.nim
+++ b/src/display/client.nim
@@ -537,6 +537,7 @@ proc sleep(client: Client, millis: int) {.jsfunc.} =
 
 proc newClient*(config: Config, dispatcher: Dispatcher): Client =
   new(result)
+  setControlCHook(proc() {.noconv.} = quit(1))
   result.config = config
   result.dispatcher = dispatcher
   result.attrs = getWindowAttributes(stdout)
diff --git a/src/ips/editor.nim b/src/ips/editor.nim
index 0e2c91c8..5a172f79 100644
--- a/src/ips/editor.nim
+++ b/src/ips/editor.nim
@@ -1,4 +1,5 @@
 import os
+import posix
 
 import config/config
 import display/term
@@ -29,6 +30,9 @@ func formatEditorName(editor, file: string, line: int): string =
       result &= ' '
     result &= file
 
+proc c_system(cmd: cstring): cint {.
+  importc: "system", header: "<stdlib.h>".}
+
 proc openEditor*(term: Terminal, config: Config, file: string, line = 1): bool =
   var editor = config.external.editor
   if editor == "":
@@ -37,7 +41,15 @@ proc openEditor*(term: Terminal, config: Config, file: string, line = 1): bool =
       editor = "vi %s +%d"
   let cmd = formatEditorName(editor, file, line)
   term.quit()
-  result = execShellCmd(cmd) == 0
+  let wstatus = c_system(cstring(cmd))
+  result = WIFEXITED(wstatus) and WEXITSTATUS(wstatus) == 0
+  if not result:
+    # Hack.
+    #TODO this is a very bad idea, e.g. say the editor is writing into the
+    # file, then receives SIGINT, now the file is corrupted but Chawan will
+    # happily read it as if nothing happened.
+    # We should find a proper solution for this.
+    result = WIFSIGNALED(wstatus) and WTERMSIG(wstatus) == SIGINT
   term.restart()
 
 var tmpf_seq: int
diff --git a/src/ips/forkserver.nim b/src/ips/forkserver.nim
index 646d7c39..1a5c2ea0 100644
--- a/src/ips/forkserver.nim
+++ b/src/ips/forkserver.nim
@@ -60,6 +60,13 @@ proc removeChild*(forkserver: Forkserver, pid: Pid) =
   forkserver.ostream.swrite(pid)
   forkserver.ostream.flush()
 
+proc trapSIGINT() =
+  # trap SIGINT, so e.g. an external editor receiving an interrupt in the
+  # same process group can't just kill the process
+  # Note that the main process normally quits on interrupt (thus terminating
+  # all child processes as well).
+  setControlCHook(proc() {.noconv.} = discard)
+
 proc forkLoader(ctx: var ForkServerContext, config: LoaderConfig): Pid =
   var pipefd: array[2, cint]
   if pipe(pipefd) == -1:
@@ -67,6 +74,7 @@ proc forkLoader(ctx: var ForkServerContext, config: LoaderConfig): Pid =
   let pid = fork()
   if pid == 0:
     # child process
+    trapSIGINT()
     for i in 0 ..< ctx.children.len: ctx.children[i] = (Pid(0), Pid(0))
     ctx.children.setLen(0)
     zeroMem(addr ctx, sizeof(ctx))
@@ -109,9 +117,11 @@ proc forkBuffer(ctx: var ForkServerContext): Pid =
     )
   )
   let pid = fork()
-  #if pid == -1:
-  #  raise newException(Defect, "Failed to fork process.")
+  if pid == -1:
+    raise newException(Defect, "Failed to fork process.")
   if pid == 0:
+    # child process
+    trapSIGINT()
     for i in 0 ..< ctx.children.len: ctx.children[i] = (Pid(0), Pid(0))
     ctx.children.setLen(0)
     zeroMem(addr ctx, sizeof(ctx))
@@ -191,6 +201,7 @@ proc newForkServer*(): ForkServer =
     raise newException(Defect, "Failed to fork the fork process.")
   elif pid == 0:
     # child process
+    trapSIGINT()
     discard close(pipefd_in[1]) # close write
     discard close(pipefd_out[0]) # close read
     discard close(pipefd_err[0]) # close read