From c9f72cbb8aa3e39c41cf125f17346193e982b21d Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Fri, 3 Mar 2023 00:41:33 +0800 Subject: [PATCH 1/4] Implement `Process.ppid` on Windows --- spec/std/process_spec.cr | 12 +++++++----- src/crystal/system/win32/process.cr | 19 ++++++++++++++++++- src/lib_c/x86_64-windows-msvc/c/tlhelp32.cr | 20 ++++++++++++++++++++ src/process.cr | 4 ++++ 4 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 src/lib_c/x86_64-windows-msvc/c/tlhelp32.cr diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index 349281ec16e8..270bf8cb237f 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -357,11 +357,13 @@ describe Process do typeof(Process.new(*standing_command).terminate(graceful: false)) pending_win32 ".exists?" do - # We can't reliably check whether it ever returns false, since we can't predict - # how PIDs are used by the system, a new process might be spawned in between - # reaping the one we would spawn and checking for it, using the now available - # pid. - Process.exists?(Process.ppid).should be_true + {% unless flag?(:win32) %} + # We can't reliably check whether it ever returns false, since we can't predict + # how PIDs are used by the system, a new process might be spawned in between + # reaping the one we would spawn and checking for it, using the now available + # pid. + Process.exists?(Process.ppid).should be_true + {% end %} process = Process.new(*standing_command) process.exists?.should be_true diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 83ae2a110f34..468f2e4ef021 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -1,6 +1,7 @@ require "c/processthreadsapi" require "c/handleapi" require "c/synchapi" +require "c/tlhelp32" require "process/shell" require "crystal/atomic_semaphore" @@ -71,7 +72,23 @@ struct Crystal::System::Process end def self.ppid - raise NotImplementedError.new("Process.ppid") + pid = self.pid + each_entry do |pe| + return pe.th32ParentProcessID if pe.th32ProcessID == pid + end + raise RuntimeError.new("Cannot locate current process") + end + + private def self.each_entry(&) + h = LibC.CreateToolhelp32Snapshot(LibC::TH32CS_SNAPPROCESS, 0) + pe = LibC::PROCESSENTRY32W.new(dwSize: sizeof(LibC::PROCESSENTRY32W)) + + if LibC.Process32FirstW(h, pointerof(pe)) != 0 + while true + yield pe + break if LibC.Process32NextW(h, pointerof(pe)) == 0 + end + end end def self.signal(pid, signal) diff --git a/src/lib_c/x86_64-windows-msvc/c/tlhelp32.cr b/src/lib_c/x86_64-windows-msvc/c/tlhelp32.cr new file mode 100644 index 000000000000..acfab00321f5 --- /dev/null +++ b/src/lib_c/x86_64-windows-msvc/c/tlhelp32.cr @@ -0,0 +1,20 @@ +lib LibC + TH32CS_SNAPPROCESS = 0x00000002 + + struct PROCESSENTRY32W + dwSize : DWORD + cntUsage : DWORD + th32ProcessID : DWORD + th32DefaultHeapID : ULONG_PTR + th32ModuleID : DWORD + cntThreads : DWORD + th32ParentProcessID : DWORD + pcPriClassBase : LONG + dwFlags : DWORD + szExeFile : WCHAR[MAX_PATH] + end + + fun CreateToolhelp32Snapshot(dwFlags : DWORD, th32ProcessID : DWORD) : HANDLE + fun Process32FirstW(hSnapshot : HANDLE, lppe : PROCESSENTRY32W*) : BOOL + fun Process32NextW(hSnapshot : HANDLE, lppe : PROCESSENTRY32W*) : BOOL +end diff --git a/src/process.cr b/src/process.cr index 69d3953e6a7a..0a14154bcf05 100644 --- a/src/process.cr +++ b/src/process.cr @@ -37,6 +37,10 @@ class Process end # Returns the process identifier of the parent process of the current process. + # + # On Windows, the parent is associated only at process creation time, and the + # system does not re-parent the current process if the parent terminates; thus + # `Process.exists?(Process.ppid)` is not guaranteed to be true. def self.ppid : Int64 Crystal::System::Process.ppid.to_i64 end From f779198010f6335d57f3bce62b172a0639ea0f66 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Fri, 3 Mar 2023 00:59:03 +0800 Subject: [PATCH 2/4] close handle --- src/crystal/system/win32/process.cr | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 468f2e4ef021..f55c4254db2f 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -81,13 +81,18 @@ struct Crystal::System::Process private def self.each_entry(&) h = LibC.CreateToolhelp32Snapshot(LibC::TH32CS_SNAPPROCESS, 0) - pe = LibC::PROCESSENTRY32W.new(dwSize: sizeof(LibC::PROCESSENTRY32W)) + raise RuntimeError.from_winerror("CreateToolhelp32Snapshot") if h == LibC::INVALID_HANDLE_VALUE - if LibC.Process32FirstW(h, pointerof(pe)) != 0 - while true - yield pe - break if LibC.Process32NextW(h, pointerof(pe)) == 0 + begin + pe = LibC::PROCESSENTRY32W.new(dwSize: sizeof(LibC::PROCESSENTRY32W)) + if LibC.Process32FirstW(h, pointerof(pe)) != 0 + while true + yield pe + break if LibC.Process32NextW(h, pointerof(pe)) == 0 + end end + ensure + LibC.CloseHandle(h) end end From 900de11ee62d7d914f6e4b4fa2faa7ee3b056c40 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 14 Mar 2023 17:29:04 +0800 Subject: [PATCH 3/4] fix spec --- spec/std/process_spec.cr | 14 +++++++++++--- src/crystal/system/win32/process.cr | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index 627c6a5f8988..fff7d26169b4 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -358,7 +358,9 @@ describe Process do typeof(Process.new(*standing_command).terminate(graceful: false)) - pending_win32 ".exists?" do + it ".exists?" do + # On Windows killing a parent process does not reparent its children to + # another existing process, so the following isn't guaranteed to work {% unless flag?(:win32) %} # We can't reliably check whether it ever returns false, since we can't predict # how PIDs are used by the system, a new process might be spawned in between @@ -373,8 +375,14 @@ describe Process do # Kill, zombie now process.terminate - process.exists?.should be_true - process.terminated?.should be_false + {% if flag?(:win32) %} + # Windows has no concept of zombie processes + process.exists?.should be_false + process.terminated?.should be_true + {% else %} + process.exists?.should be_true + process.terminated?.should be_false + {% end %} # Reap, gone now process.wait diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index f55c4254db2f..d803199e2e26 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -151,7 +151,7 @@ struct Crystal::System::Process def self.exists?(pid) handle = LibC.OpenProcess(LibC::PROCESS_QUERY_INFORMATION, 0, pid) - return false if handle.nil? + return false unless handle begin if LibC.GetExitCodeProcess(handle, out exit_code) == 0 raise RuntimeError.from_winerror("GetExitCodeProcess") From beabef17d7f4ae8730ebaba0c6e2e088d59f4e3d Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 14 Mar 2023 18:00:38 +0800 Subject: [PATCH 4/4] fixup --- src/crystal/system/win32/process.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index d803199e2e26..b887d8521c46 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -73,13 +73,13 @@ struct Crystal::System::Process def self.ppid pid = self.pid - each_entry do |pe| + each_process_entry do |pe| return pe.th32ParentProcessID if pe.th32ProcessID == pid end raise RuntimeError.new("Cannot locate current process") end - private def self.each_entry(&) + private def self.each_process_entry(&) h = LibC.CreateToolhelp32Snapshot(LibC::TH32CS_SNAPPROCESS, 0) raise RuntimeError.from_winerror("CreateToolhelp32Snapshot") if h == LibC::INVALID_HANDLE_VALUE