From 11053afff83e00bb04be05590a169ed79ce5f0d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zieli=C5=84ski?= Date: Sun, 26 Jan 2014 22:30:20 +0100 Subject: [PATCH 1/4] osproc: fix naming inconsistiences --- lib/pure/osproc.nim | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index c7678b2148..f764971c4a 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -13,7 +13,7 @@ include "system/inclrtl" import - strutils, os, strtabs, streams, sequtils + strutils, os, strtabs, streams when defined(windows): import winlean @@ -44,7 +44,7 @@ type poStdErrToStdOut, ## merge stdout and stderr to the stdout stream poParentStreams ## use the parent's streams -template poUseShell*: TProcessOption {.deprecated.} = poUsePath +const poUseShell* {.deprecated.} = poUsePath ## Deprecated alias for poUsePath. proc quoteShellWindows*(s: string): string {.noSideEffect, rtl, extern: "nosp$1".} = @@ -604,20 +604,20 @@ elif not defined(useNimRtl): pipe(pStderr) != 0'i32: osError(osLastError()) - var sys_command: string - var sys_args_raw: seq[string] + var sysCommand: string + var sysArgsRaw: seq[string] if poEvalCommand in options: - sys_command = "/bin/sh" - sys_args_raw = @[sys_command, "-c", command] + sysCommand = "/bin/sh" + sysArgsRaw = @[sysCommand, "-c", command] assert args.len == 0 else: - sys_command = command - sys_args_raw = @[command] + sysCommand = command + sysArgsRaw = @[command] for arg in args.items: - sys_args_raw.add arg + sysArgsRaw.add arg - var sys_args = allocCStringArray(sys_args_raw) - finally: deallocCStringArray(sys_args) + var sysArgs = allocCStringArray(sysArgsRaw) + finally: deallocCStringArray(sysArgs) var pid: TPid when defined(posix_spawn) and not defined(useFork): @@ -650,15 +650,15 @@ elif not defined(useNimRtl): else: chck posix_spawn_file_actions_adddup2(fops, p_stderr[writeIdx], 2) - var sys_env = if env == nil: envToCStringArray() else: envToCStringArray(env) + var sysEnv = if env == nil: envToCStringArray() else: envToCStringArray(env) var res: cint - # This is incorrect! + # FIXME: chdir is global to process if workingDir.len > 0: os.setCurrentDir(workingDir) if poUsePath in options: - res = posix_spawnp(pid, sys_command, fops, attr, sys_args, sys_env) + res = posix_spawnp(pid, sysCommand, fops, attr, sysArgs, sysEnv) else: - res = posix_spawn(pid, sys_command, fops, attr, sys_args, sys_env) - deallocCStringArray(sys_env) + res = posix_spawn(pid, sysCommand, fops, attr, sysArgs, sysEnv) + deallocCStringArray(sysEnv) discard posix_spawn_file_actions_destroy(fops) discard posix_spawnattr_destroy(attr) chck res @@ -687,15 +687,15 @@ elif not defined(useNimRtl): if env == nil: if poUsePath in options: - discard execvp(sys_command, sys_args) + discard execvp(sysCommand, sysArgs) else: - discard execv(sys_command, sys_args) + discard execv(sysCommand, sysArgs) else: - var c_env = envToCStringArray(env) + var cEnv = envToCStringArray(env) if poUsePath in options: - discard execvpe(sys_command, sys_args, c_env) + discard execvpe(sysCommand, sysArgs, cEnv) else: - discard execve(sys_command, sys_args, c_env) + discard execve(sysCommand, sysArgs, cEnv) # too risky to raise an exception here: quit("execve call failed: " & $strerror(errno)) # Parent process. Copy process information. From cd2bd7fa7b1048956c72ad7665b70b2eabecd549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zieli=C5=84ski?= Date: Fri, 14 Feb 2014 15:08:02 +0100 Subject: [PATCH 2/4] osproc: use clone with CLONE_VM on Linux for faster process spawning --- lib/posix/linux.nim | 25 ++++++ lib/pure/osproc.nim | 193 +++++++++++++++++++++++++++----------------- 2 files changed, 146 insertions(+), 72 deletions(-) create mode 100644 lib/posix/linux.nim diff --git a/lib/posix/linux.nim b/lib/posix/linux.nim new file mode 100644 index 0000000000..1ed1af3b6d --- /dev/null +++ b/lib/posix/linux.nim @@ -0,0 +1,25 @@ +import posix + +const + CSIGNAL* = 0x000000FF + CLONE_VM* = 0x00000100 + CLONE_FS* = 0x00000200 + CLONE_FILES* = 0x00000400 + CLONE_SIGHAND* = 0x00000800 + CLONE_PTRACE* = 0x00002000 + CLONE_VFORK* = 0x00004000 + CLONE_PARENT* = 0x00008000 + CLONE_THREAD* = 0x00010000 + CLONE_NEWNS* = 0x00020000 + CLONE_SYSVSEM* = 0x00040000 + CLONE_SETTLS* = 0x00080000 + CLONE_PARENT_SETTID* = 0x00100000 + CLONE_CHILD_CLEARTID* = 0x00200000 + CLONE_DETACHED* = 0x00400000 + CLONE_UNTRACED* = 0x00800000 + CLONE_CHILD_SETTID* = 0x01000000 + CLONE_STOPPED* = 0x02000000 + +# fn should be of type proc (a2: pointer): void {.cdecl.} +proc clone*(fn: pointer; child_stack: pointer; flags: cint; + arg: pointer; ptid: ptr TPid; tls: pointer; ctid: ptr TPid): cint {.importc, header: "".} diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index c0c7f46d74..40877f6382 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -20,6 +20,11 @@ when defined(windows): else: import posix +when defined(linux): + import linux + when not defined(useFork): + const useClone = true + type TProcess = object of TObject when defined(windows): @@ -590,6 +595,19 @@ elif not defined(useNimRtl): copyMem(result[i], addr(x[0]), x.len+1) inc(i) + type TStartProcessData = object + sysCommand: cstring + sysArgs: cstringArray + sysEnv: cstringArray + workingDir: cstring + pStdin, pStdout, pStderr: array[0..1, cint] + optionPoUsePath: bool + optionPoParentStreams: bool + optionPoStdErrToStdOut: bool + + proc startProcessAuxSpawn(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].} + proc startProcessAfterFork(data: ptr TStartProcessData) {.tags: [FExecIO, FReadEnv], cdecl.} + proc startProcess(command: string, workingDir: string = "", args: openArray[string] = [], @@ -616,88 +634,48 @@ elif not defined(useNimRtl): for arg in args.items: sysArgsRaw.add arg + var pid: TPid + var sysArgs = allocCStringArray(sysArgsRaw) finally: deallocCStringArray(sysArgs) - var pid: TPid - when defined(posix_spawn) and not defined(useFork): - var attr: Tposix_spawnattr - var fops: Tposix_spawn_file_actions - - template chck(e: expr) = - if e != 0'i32: osError(osLastError()) - - chck posix_spawn_file_actions_init(fops) - chck posix_spawnattr_init(attr) - - var mask: Tsigset - chck sigemptyset(mask) - chck posix_spawnattr_setsigmask(attr, mask) - chck posix_spawnattr_setpgroup(attr, 0'i32) - - chck posix_spawnattr_setflags(attr, POSIX_SPAWN_USEVFORK or - POSIX_SPAWN_SETSIGMASK or - POSIX_SPAWN_SETPGROUP) - - if poParentStreams notin options: - chck posix_spawn_file_actions_addclose(fops, pStdin[writeIdx]) - chck posix_spawn_file_actions_adddup2(fops, pStdin[readIdx], readIdx) - chck posix_spawn_file_actions_addclose(fops, pStdout[readIdx]) - chck posix_spawn_file_actions_adddup2(fops, pStdout[writeIdx], writeIdx) - chck posix_spawn_file_actions_addclose(fops, pStderr[readIdx]) - if poStdErrToStdOut in options: - chck posix_spawn_file_actions_adddup2(fops, pStdout[writeIdx], 2) - else: - chck posix_spawn_file_actions_adddup2(fops, p_stderr[writeIdx], 2) - - var sysEnv = if env == nil: envToCStringArray() else: envToCStringArray(env) - var res: cint - # FIXME: chdir is global to process - if workingDir.len > 0: os.setCurrentDir(workingDir) - if poUsePath in options: - res = posix_spawnp(pid, sysCommand, fops, attr, sysArgs, sysEnv) + var sysEnv = if env == nil: + envToCStringArray() else: - res = posix_spawn(pid, sysCommand, fops, attr, sysArgs, sysEnv) - deallocCStringArray(sysEnv) - discard posix_spawn_file_actions_destroy(fops) - discard posix_spawnattr_destroy(attr) - chck res + envToCStringArray(env) + finally: deallocCStringArray(sysEnv) + + var data: TStartProcessData + data.sysCommand = sysCommand + data.sysArgs = sysArgs + data.sysEnv = sysEnv + data.pStdin = pStdin + data.pStdout = pStdout + data.pStderr = pStderr + data.optionPoParentStreams = poParentStreams in options + data.optionPoUsePath = poUsePath in options + data.optionPoStdErrToStdOut = poStdErrToStdOut in options + data.workingDir = workingDir + + when defined(useClone): + const stackSize = 8096 + let stackEnd = cast[clong](alloc(stackSize)) + let stack = cast[pointer](stackEnd + stackSize) + let fn: pointer = startProcessAfterFork + pid = clone(fn, stack, + cint(CLONE_VM or CLONE_VFORK or SIGCHLD), + pointer(addr data), nil, nil, nil) + + if pid < 0: osError(osLastError()) + elif defined(posix_spawn) and not defined(useFork): + pid = startProcessAuxSpawn(data) else: pid = fork() if pid < 0: osError(osLastError()) if pid == 0: - ## child process: + startProcessAfterFork(addr(data)) - if poParentStreams notin options: - discard close(p_stdin[writeIdx]) - if dup2(p_stdin[readIdx], readIdx) < 0: osError(osLastError()) - discard close(p_stdout[readIdx]) - if dup2(p_stdout[writeIdx], writeIdx) < 0: osError(osLastError()) - discard close(p_stderr[readIdx]) - if poStdErrToStdOut in options: - if dup2(p_stdout[writeIdx], 2) < 0: osError(osLastError()) - else: - if dup2(p_stderr[writeIdx], 2) < 0: osError(osLastError()) - - # Create a new process group - if setpgid(0, 0) == -1: quit("setpgid call failed: " & $strerror(errno)) - - if workingDir.len > 0: os.setCurrentDir(workingDir) - - if env == nil: - if poUsePath in options: - discard execvp(sysCommand, sysArgs) - else: - discard execv(sysCommand, sysArgs) - else: - var cEnv = envToCStringArray(env) - if poUsePath in options: - discard execvpe(sysCommand, sysArgs, cEnv) - else: - discard execve(sysCommand, sysArgs, cEnv) - # too risky to raise an exception here: - quit("execve call failed: " & $strerror(errno)) # Parent process. Copy process information. if poEchoCmd in options: echo(command, " ", join(args, " ")) @@ -723,6 +701,77 @@ elif not defined(useNimRtl): discard close(pStdin[readIdx]) discard close(pStdout[writeIdx]) + proc startProcessAuxSpawn(data: TStartProcessData): TPid = + var attr: Tposix_spawnattr + var fops: Tposix_spawn_file_actions + + template chck(e: expr) = + if e != 0'i32: osError(osLastError()) + + chck posix_spawn_file_actions_init(fops) + chck posix_spawnattr_init(attr) + + var mask: Tsigset + chck sigemptyset(mask) + chck posix_spawnattr_setsigmask(attr, mask) + chck posix_spawnattr_setpgroup(attr, 0'i32) + + chck posix_spawnattr_setflags(attr, POSIX_SPAWN_USEVFORK or + POSIX_SPAWN_SETSIGMASK or + POSIX_SPAWN_SETPGROUP) + + if not data.optionPoParentStreams: + chck posix_spawn_file_actions_addclose(fops, data.pStdin[writeIdx]) + chck posix_spawn_file_actions_adddup2(fops, data.pStdin[readIdx], readIdx) + chck posix_spawn_file_actions_addclose(fops, data.pStdout[readIdx]) + chck posix_spawn_file_actions_adddup2(fops, data.pStdout[writeIdx], writeIdx) + chck posix_spawn_file_actions_addclose(fops, data.pStderr[readIdx]) + if data.optionPoStdErrToStdOut: + chck posix_spawn_file_actions_adddup2(fops, data.pStdout[writeIdx], 2) + else: + chck posix_spawn_file_actions_adddup2(fops, data.pStderr[writeIdx], 2) + + var res: cint + # FIXME: chdir is global to process + if data.workingDir.len > 0: + setCurrentDir($data.workingDir) + var pid: TPid + + if data.optionPoUsePath: + res = posix_spawnp(pid, data.sysCommand, fops, attr, data.sysArgs, data.sysEnv) + else: + res = posix_spawn(pid, data.sysCommand, fops, attr, data.sysArgs, data.sysEnv) + + discard posix_spawn_file_actions_destroy(fops) + discard posix_spawnattr_destroy(attr) + chck res + return pid + + proc startProcessAfterFork(data: ptr TStartProcessData) = + # Warning: no GC here! + if not data.optionPoParentStreams: + discard close(data.pStdin[writeIdx]) + if dup2(data.pStdin[readIdx], readIdx) < 0: osError(osLastError()) + discard close(data.pStdout[readIdx]) + if dup2(data.pStdout[writeIdx], writeIdx) < 0: osError(osLastError()) + discard close(data.pStderr[readIdx]) + if data.optionPoStdErrToStdOut: + if dup2(data.pStdout[writeIdx], 2) < 0: osError(osLastError()) + else: + if dup2(data.pStderr[writeIdx], 2) < 0: osError(osLastError()) + + if data.workingDir.len > 0: + if chdir(data.workingDir) < 0: + quit("chdir failed") + + if data.optionPoUsePath: + discard execvpe(data.sysCommand, data.sysArgs, data.sysEnv) + else: + discard execve(data.sysCommand, data.sysArgs, data.sysEnv) + + # too risky to raise an exception here: + quit("execve call failed: " & $strerror(errno)) + proc close(p: PProcess) = if p.inStream != nil: close(p.inStream) if p.outStream != nil: close(p.outStream) From 4c09fc110f3d269c34ccbfabb665bc34c768b63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zieli=C5=84ski?= Date: Fri, 14 Feb 2014 15:51:06 +0100 Subject: [PATCH 3/4] osproc: make failed execv an exception (when using fork or clone) startProcessAuxFork creates a pipe, which is used by a child to pass an error code if execv fails. --- lib/posix/posix.nim | 1 + lib/pure/osproc.nim | 86 +++++++++++++++++++++++++++++++++------------ 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/lib/posix/posix.nim b/lib/posix/posix.nim index 41260b36fc..138df1aecf 100644 --- a/lib/posix/posix.nim +++ b/lib/posix/posix.nim @@ -2066,6 +2066,7 @@ proc pthread_spin_unlock*(a1: ptr Tpthread_spinlock): cint {. proc pthread_testcancel*() {.importc, header: "".} +proc exitnow*(code: int): void {.importc: "_exit", header: "".} proc access*(a1: cstring, a2: cint): cint {.importc, header: "".} proc alarm*(a1: cint): cint {.importc, header: "".} proc chdir*(a1: cstring): cint {.importc, header: "".} diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index 40877f6382..aa2f6f937d 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -600,13 +600,16 @@ elif not defined(useNimRtl): sysArgs: cstringArray sysEnv: cstringArray workingDir: cstring - pStdin, pStdout, pStderr: array[0..1, cint] + pStdin, pStdout, pStderr, pErrorPipe: array[0..1, cint] optionPoUsePath: bool optionPoParentStreams: bool optionPoStdErrToStdOut: bool proc startProcessAuxSpawn(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].} - proc startProcessAfterFork(data: ptr TStartProcessData) {.tags: [FExecIO, FReadEnv], cdecl.} + + proc startProcessAuxFork(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].} + proc startProcessAfterFork(data: ptr TStartProcessData) {. + tags: [FExecIO, FReadEnv], noStackFrame, cdecl.} proc startProcess(command: string, workingDir: string = "", @@ -658,23 +661,11 @@ elif not defined(useNimRtl): data.optionPoStdErrToStdOut = poStdErrToStdOut in options data.workingDir = workingDir - when defined(useClone): - const stackSize = 8096 - let stackEnd = cast[clong](alloc(stackSize)) - let stack = cast[pointer](stackEnd + stackSize) - let fn: pointer = startProcessAfterFork - pid = clone(fn, stack, - cint(CLONE_VM or CLONE_VFORK or SIGCHLD), - pointer(addr data), nil, nil, nil) - if pid < 0: osError(osLastError()) - elif defined(posix_spawn) and not defined(useFork): + when defined(posix_spawn) and not defined(useFork) and not defined(useClone): pid = startProcessAuxSpawn(data) else: - pid = fork() - if pid < 0: osError(osLastError()) - if pid == 0: - startProcessAfterFork(addr(data)) + pid = startProcessAuxFork(data) # Parent process. Copy process information. if poEchoCmd in options: @@ -747,30 +738,79 @@ elif not defined(useNimRtl): chck res return pid + proc startProcessAuxFork(data: TStartProcessData): TPid = + if pipe(data.pErrorPipe) != 0: + osError(osLastError()) + + finally: + discard close(data.pErrorPipe[readIdx]) + + var pid: TPid + var dataCopy = data + + if defined(useClone): + const stackSize = 8096 + let stackEnd = cast[clong](alloc(stackSize)) + let stack = cast[pointer](stackEnd + stackSize) + let fn: pointer = startProcessAfterFork + pid = clone(fn, stack, + cint(CLONE_VM or CLONE_VFORK or SIGCHLD), + pointer(addr dataCopy), nil, nil, nil) + discard close(data.pErrorPipe[writeIdx]) + dealloc(stack) + else: + pid = fork() + if pid == 0: + startProcessAfterFork(addr(dataCopy)) + exitnow(1) + + discard close(data.pErrorPipe[writeIdx]) + if pid < 0: osError(osLastError()) + + var error: cint + let sizeRead = read(data.pErrorPipe[readIdx], addr error, sizeof(error)) + if sizeRead == sizeof(error): + osError($strerror(error)) + + return pid + + proc startProcessFail(data: ptr TStartProcessData) {.noStackFrame.} = + var error: cint = errno + discard write(data.pErrorPipe[writeIdx], addr error, sizeof(error)) + exitnow(1) + proc startProcessAfterFork(data: ptr TStartProcessData) = # Warning: no GC here! + # Or anythink that touches global structures - all called nimrod procs + # must be marked with noStackFrame. Inspect C code after making changes. if not data.optionPoParentStreams: discard close(data.pStdin[writeIdx]) - if dup2(data.pStdin[readIdx], readIdx) < 0: osError(osLastError()) + if dup2(data.pStdin[readIdx], readIdx) < 0: + startProcessFail(data) discard close(data.pStdout[readIdx]) - if dup2(data.pStdout[writeIdx], writeIdx) < 0: osError(osLastError()) + if dup2(data.pStdout[writeIdx], writeIdx) < 0: + startProcessFail(data) discard close(data.pStderr[readIdx]) if data.optionPoStdErrToStdOut: - if dup2(data.pStdout[writeIdx], 2) < 0: osError(osLastError()) + if dup2(data.pStdout[writeIdx], 2) < 0: + startProcessFail(data) else: - if dup2(data.pStderr[writeIdx], 2) < 0: osError(osLastError()) + if dup2(data.pStderr[writeIdx], 2) < 0: + startProcessFail(data) if data.workingDir.len > 0: if chdir(data.workingDir) < 0: - quit("chdir failed") + startProcessFail(data) + + discard close(data.pErrorPipe[readIdx]) + discard fcntl(data.pErrorPipe[writeIdx], F_SETFD, FD_CLOEXEC) if data.optionPoUsePath: discard execvpe(data.sysCommand, data.sysArgs, data.sysEnv) else: discard execve(data.sysCommand, data.sysArgs, data.sysEnv) - # too risky to raise an exception here: - quit("execve call failed: " & $strerror(errno)) + startProcessFail(data) proc close(p: PProcess) = if p.inStream != nil: close(p.inStream) From 1f376d8594d9e20aa20b900853a166cbb49af5bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zieli=C5=84ski?= Date: Tue, 18 Feb 2014 20:22:40 +0100 Subject: [PATCH 4/4] osproc: use push stacktrace:off instead of nostackframe --- lib/pure/osproc.nim | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index aa2f6f937d..93d737aa69 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -606,10 +606,11 @@ elif not defined(useNimRtl): optionPoStdErrToStdOut: bool proc startProcessAuxSpawn(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].} - proc startProcessAuxFork(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].} + {.push stacktrace: off, profiler: off.} proc startProcessAfterFork(data: ptr TStartProcessData) {. - tags: [FExecIO, FReadEnv], noStackFrame, cdecl.} + tags: [FExecIO, FReadEnv], cdecl.} + {.pop.} proc startProcess(command: string, workingDir: string = "", @@ -774,7 +775,8 @@ elif not defined(useNimRtl): return pid - proc startProcessFail(data: ptr TStartProcessData) {.noStackFrame.} = + {.push stacktrace: off, profiler: off.} + proc startProcessFail(data: ptr TStartProcessData) = var error: cint = errno discard write(data.pErrorPipe[writeIdx], addr error, sizeof(error)) exitnow(1) @@ -811,6 +813,7 @@ elif not defined(useNimRtl): discard execve(data.sysCommand, data.sysArgs, data.sysEnv) startProcessFail(data) + {.pop} proc close(p: PProcess) = if p.inStream != nil: close(p.inStream)