From ba4b3a12069700e58b5404e8a88bad977489418f Mon Sep 17 00:00:00 2001 From: "Hristo (Izo) Gueorguiev" <53634432+izo0x90@users.noreply.github.com> Date: Sat, 15 Feb 2025 08:03:58 -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 | 95 +------------------------------- stdlib/src/sys/_libc.mojo | 8 +-- stdlib/src/sys/ffi.mojo | 2 +- stdlib/test/os/test_process.mojo | 28 +++++----- 5 files changed, 21 insertions(+), 114 deletions(-) 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 f2c1fd3fb5..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,94 +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 - """Child process id.""" - - fn __init__(mut self, child_pid: c_int): - """Struct to manage meta about child process. - - Args: - child_pid: The pid of child processed returned by `fork` that the struct will manage. - """ - - self.child_pid = child_pid - - fn _kill(self, signal: Int): - kill(self.child_pid, signal) - - fn hangup(self): - """Send the Hang up signal to the managed child process.""" - self._kill(SignalCodes.HUP) - - fn interrupt(self): - """Send the Interrupt signal to the managed child process.""" - self._kill(SignalCodes.INT) - - fn kill(self): - """Send the Kill signal to the managed child process.""" - 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/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()