diff options
author | bptato <nincsnevem662@gmail.com> | 2023-06-25 22:53:57 +0200 |
---|---|---|
committer | bptato <nincsnevem662@gmail.com> | 2023-06-26 00:46:53 +0200 |
commit | d0c4570fffec4c4d9db909f59b7f70c89e012acf (patch) | |
tree | 2657c9686c2854f5cc2a5486a80b88a788b2266d | |
parent | 0524bed395cfeb467812854c55673c8dc87a1bde (diff) | |
download | chawan-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.toml | 1 | ||||
-rw-r--r-- | src/config/config.nim | 1 | ||||
-rw-r--r-- | src/display/client.nim | 1 | ||||
-rw-r--r-- | src/ips/editor.nim | 14 | ||||
-rw-r--r-- | src/ips/forkserver.nim | 15 |
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 |