From 6342634adac0641f5b7b2d0e062282c6ff89bbfa Mon Sep 17 00:00:00 2001 From: "Hristo (Izo) Gueorguiev" <53634432+izo0x90@users.noreply.github.com> Date: Fri, 14 Feb 2025 17:44:19 -0500 Subject: [PATCH] Fixes/ updates to `process` functionality - Move `process` related code to its own file - Use `vfork` to avoid issues with threading until they are resolved in Mojo - Sending signals to process returns success status - Docstr additions --- stdlib/src/os/__init__.mojo | 2 +- stdlib/src/os/os.mojo | 85 +--------------------- stdlib/src/os/process.mojo | 121 +++++++++++++++++++++++++++++++ stdlib/src/sys/_libc.mojo | 8 +- stdlib/src/sys/ffi.mojo | 2 +- stdlib/test/os/test_process.mojo | 28 +++---- 6 files changed, 142 insertions(+), 104 deletions(-) create mode 100644 stdlib/src/os/process.mojo diff --git a/stdlib/src/os/__init__.mojo b/stdlib/src/os/__init__.mojo index 443700f357..21c22ace7a 100644 --- a/stdlib/src/os/__init__.mojo +++ b/stdlib/src/os/__init__.mojo @@ -19,7 +19,6 @@ from .os import ( SEEK_CUR, SEEK_END, SEEK_SET, - Process, abort, getuid, listdir, @@ -32,3 +31,4 @@ from .os import ( unlink, ) from .pathlike import PathLike +from .process import Process diff --git a/stdlib/src/os/os.mojo b/stdlib/src/os/os.mojo index 99682f276a..d6c9cecbbf 100644 --- a/stdlib/src/os/os.mojo +++ b/stdlib/src/os/os.mojo @@ -25,11 +25,9 @@ from sys import ( external_call, is_gpu, os_is_linux, - os_is_macos, os_is_windows, ) -from sys._libc import fork, execvp, kill, SignalCodes -from sys.ffi import OpaquePointer, c_char, c_int, c_str_ptr +from sys.ffi import OpaquePointer, c_char from memory import UnsafePointer @@ -422,84 +420,3 @@ def removedirs[PathLike: os.PathLike](path: PathLike) -> None: except: break head, tail = os.path.split(head) - - -# ===----------------------------------------------------------------------=== # -# Process execution -# ===----------------------------------------------------------------------=== # - - -struct Process: - """Create and manage child processes from file executables. - TODO: Add windows support. - """ - - var child_pid: c_int - - fn __init__(mut self, child_pid: c_int): - self.child_pid = child_pid - - fn _kill(self, signal: Int): - kill(self.child_pid, signal) - - fn hangup(self): - self._kill(SignalCodes.HUP) - - fn interrupt(self): - self._kill(SignalCodes.INT) - - fn kill(self): - self._kill(SignalCodes.KILL) - - @staticmethod - fn run(path: String, argv: List[String]) raises -> Process: - """Spawn new process from file executable. - - Args: - path: The path to the file. - argv: A list of string arguments to be passed to executable. - - Returns: - An instance of `Process` struct. - """ - - @parameter - if os_is_linux() or os_is_macos(): - var file_name = path.split(sep)[-1] - var pid = fork() - if pid == 0: - var arg_count = len(argv) - var argv_array_ptr_cstr_ptr = UnsafePointer[c_str_ptr].alloc( - arg_count + 2 - ) - var offset = 0 - # Arg 0 in `argv` ptr array should be the file name - argv_array_ptr_cstr_ptr[offset] = file_name.unsafe_cstr_ptr() - offset += 1 - - for arg in argv: - argv_array_ptr_cstr_ptr[offset] = arg[].unsafe_cstr_ptr() - offset += 1 - - # `argv` ptr array terminates with NULL PTR - argv_array_ptr_cstr_ptr[offset] = c_str_ptr() - - _ = execvp(path.unsafe_cstr_ptr(), argv_array_ptr_cstr_ptr) - - # This will only get reached if exec call fails to replace currently executing code - argv_array_ptr_cstr_ptr.free() - raise Error("Failed to execute " + path) - elif pid < 0: - raise Error("Unable to fork parent") - - return Process(child_pid=pid) - elif os_is_windows(): - constrained[ - False, "Windows process execution currently not implemented" - ]() - return abort[Process]() - else: - constrained[ - False, "Unknown platform process execution not implemented" - ]() - return abort[Process]() diff --git a/stdlib/src/os/process.mojo b/stdlib/src/os/process.mojo new file mode 100644 index 0000000000..3736d8054c --- /dev/null +++ b/stdlib/src/os/process.mojo @@ -0,0 +1,121 @@ +from sys import ( + external_call, + os_is_linux, + os_is_macos, + os_is_windows, +) +from sys._libc import vfork, execvp, kill, SignalCodes +from sys.ffi import OpaquePointer, c_char, c_int, c_str_ptr +from sys.os import sep + +from memory import UnsafePointer + +# ===----------------------------------------------------------------------=== # +# Process execution +# ===----------------------------------------------------------------------=== # + + +struct Process: + """Create and manage child processes from file executables. + + Example usage: + ``` + child_process = Process.run("ls", List[String]("-lha")) + if child_process.interrupt(): + print("Successfully interrupted.") + ``` + """ + + var child_pid: c_int + """Child process id.""" + + fn __init__(mut self, child_pid: c_int): + """Struct to manage metadata about child process. + Use the `run` static method to create new process. + + Args: + child_pid: The pid of child processed returned by `vfork` that the struct will manage. + """ + + self.child_pid = child_pid + + fn _kill(self, signal: Int) -> Bool: + # `kill` returns 0 on success and -1 on failure + return kill(self.child_pid, signal) > -1 + + fn hangup(self) -> Bool: + """Send the Hang up signal to the managed child process. + + Returns: + Upon successful completion, True is returned else False. + """ + return self._kill(SignalCodes.HUP) + + fn interrupt(self) -> Bool: + """Send the Interrupt signal to the managed child process. + + Returns: + Upon successful completion, True is returned else False. + """ + return self._kill(SignalCodes.INT) + + fn kill(self) -> Bool: + """Send the Kill signal to the managed child process. + + Returns: + Upon successful completion, True is returned else False. + """ + return self._kill(SignalCodes.KILL) + + @staticmethod + fn run(path: String, argv: List[String]) raises -> Process: + """Spawn new process from file executable. + + Args: + path: The path to the file. + argv: A list of string arguments to be passed to executable. + + Returns: + An instance of `Process` struct. + """ + + @parameter + if os_is_linux() or os_is_macos(): + var file_name = path.split(sep)[-1] + var pid = vfork() + if pid == 0: + var arg_count = len(argv) + var argv_array_ptr_cstr_ptr = UnsafePointer[c_str_ptr].alloc( + arg_count + 2 + ) + var offset = 0 + # Arg 0 in `argv` ptr array should be the file name + argv_array_ptr_cstr_ptr[offset] = file_name.unsafe_cstr_ptr() + offset += 1 + + for arg in argv: + argv_array_ptr_cstr_ptr[offset] = arg[].unsafe_cstr_ptr() + offset += 1 + + # `argv` ptr array terminates with NULL PTR + argv_array_ptr_cstr_ptr[offset] = c_str_ptr() + + _ = execvp(path.unsafe_cstr_ptr(), argv_array_ptr_cstr_ptr) + + # This will only get reached if exec call fails to replace currently executing code + argv_array_ptr_cstr_ptr.free() + raise Error("Failed to execute " + path) + elif pid < 0: + raise Error("Unable to fork parent") + + return Process(child_pid=pid) + elif os_is_windows(): + constrained[ + False, "Windows process execution currently not implemented" + ]() + return abort[Process]() + else: + constrained[ + False, "Unknown platform process execution not implemented" + ]() + return abort[Process]() diff --git a/stdlib/src/sys/_libc.mojo b/stdlib/src/sys/_libc.mojo index e273ca617f..a5832e8d6f 100644 --- a/stdlib/src/sys/_libc.mojo +++ b/stdlib/src/sys/_libc.mojo @@ -114,8 +114,8 @@ fn execvp(file: UnsafePointer[c_char], argv: UnsafePointer[c_str_ptr]) -> c_int: @always_inline -fn fork() -> c_int: - return external_call["fork", c_int]() +fn vfork() -> c_int: + return external_call["vfork", c_int]() struct SignalCodes: @@ -129,8 +129,8 @@ struct SignalCodes: @always_inline -fn kill(pid: c_int, sig: c_int): - external_call["kill", NoneType](pid, sig) +fn kill(pid: c_int, sig: c_int) -> c_int: + return external_call["kill", c_int](pid, sig) # ===-----------------------------------------------------------------------===# diff --git a/stdlib/src/sys/ffi.mojo b/stdlib/src/sys/ffi.mojo index 7d17b64160..75210879e7 100644 --- a/stdlib/src/sys/ffi.mojo +++ b/stdlib/src/sys/ffi.mojo @@ -71,7 +71,7 @@ alias c_float = Float32 alias c_double = Float64 """C `double` type.""" -alias c_str_ptr = UnsafePointer[Int8] +alias c_str_ptr = UnsafePointer[c_char] """C `*char` type""" alias OpaquePointer = UnsafePointer[NoneType] diff --git a/stdlib/test/os/test_process.mojo b/stdlib/test/os/test_process.mojo index b076578997..f3c098bf58 100644 --- a/stdlib/test/os/test_process.mojo +++ b/stdlib/test/os/test_process.mojo @@ -25,21 +25,21 @@ def test_process_run(): _ = Process.run("echo", List[String]("== TEST")) -def test_process_run_missing(): - missing_executable_file = "ThIsFiLeCoUlDNoTPoSsIbLlYExIsT.NoTAnExTeNsIoN" - - # verify that the test file does not exist before starting the test - assert_false( - exists(missing_executable_file), - "Unexpected file '" + missing_executable_file + "' it should not exist", - ) - - # Forking appears to break asserts - with assert_raises(): - _ = Process.run(missing_executable_file, List[String]()) +# def test_process_run_missing(): +# # assert_raises does not work with exception raised in child process +# # crashes with thread error +# missing_executable_file = "ThIsFiLeCoUlDNoTPoSsIbLlYExIsT.NoTAnExTeNsIoN" +# +# # verify that the test file does not exist before starting the test +# assert_false( +# exists(missing_executable_file), +# "Unexpected file '" + missing_executable_file + "' it should not exist", +# ) +# +# # Forking appears to break asserts +# with assert_raises(): +# _ = Process.run(missing_executable_file, List[String]()) def main(): test_process_run() - # TODO: How can exception being raised on missing file be asserted - # test_process_run_missing()