diff options
author | Marc A. Paradise <marc.paradise@gmail.com> | 2022-10-06 10:25:53 -0400 |
---|---|---|
committer | Marc A. Paradise <marc.paradise@gmail.com> | 2022-10-07 17:14:12 -0400 |
commit | 7172ceba1d749404b3ad331b4a9f8b2a584ab20b (patch) | |
tree | 26855c05a280b36771123a973d33c3655f6256da | |
parent | 9c43a97b4f2722f5e88ceb290817e82c4ef336b4 (diff) | |
download | chef-7172ceba1d749404b3ad331b4a9f8b2a584ab20b.tar.gz |
Use GetCurrentProcess to determine if CloseHandle required
On Windows 2012, we are seeing frequent errors during the finalizer for
Win32::Handle, which invokes CloseHandle:
'The handle is invalid' (error code 6)
Debugging and testing has shown that every instance of this error is
caused by the handle 18446744073709551615 -> "ffffffffffffffff". We have also seen
that this matches the value passed into the constructor of Handle,
provided directly from GetCurrentHandle.
We were previously comparing this to the const "ffffffff" and skipping
the CloseHandle call on the assumption that "ffffffff" == (HANDLE)-1
based on the docs here: http://msdn.microsoft.com/en-us/library/windows/desktop/ms683179(v=vs.85).aspx
But given that we are directly seeing the GetCurrentHandle returning
"ffffffffffffffff", it seems that assumption was incorrect or at least has
changed.
Instead of worrying about the correct value of that const, this PR
makes it so that we're following the recommended approach in the link
above - if we want to check if a handle == current process handle, we
compare it to GetCurrentHandle instead of the const.
In the process, it resolves the errors we're seeing on the Windows
Server 2012 platform, where we are failing when CloseHandle returns
"invalid handle".
Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
Signed-off-by: John McCrae <john.mccrae@progress.com>
-rw-r--r-- | lib/chef/win32/handle.rb | 13 |
1 files changed, 6 insertions, 7 deletions
diff --git a/lib/chef/win32/handle.rb b/lib/chef/win32/handle.rb index 1b0257ed68..a677b4021e 100644 --- a/lib/chef/win32/handle.rb +++ b/lib/chef/win32/handle.rb @@ -26,10 +26,6 @@ class Chef class Handle extend Chef::ReservedNames::Win32::API::Process - # See http://msdn.microsoft.com/en-us/library/windows/desktop/ms683179(v=vs.85).aspx - # The handle value returned by the GetCurrentProcess function is the pseudo handle (HANDLE)-1 (which is 0xFFFFFFFF) - CURRENT_PROCESS_HANDLE = 4294967295 - def initialize(handle) @handle = handle ObjectSpace.define_finalizer(self, Handle.close_handle_finalizer(handle)) @@ -38,13 +34,16 @@ class Chef attr_reader :handle def self.close_handle_finalizer(handle) + proc { close_handle(handle) } + end + + def self.close_handle(handle) # According to http://msdn.microsoft.com/en-us/library/windows/desktop/ms683179(v=vs.85).aspx, it is not necessary # to close the pseudo handle returned by the GetCurrentProcess function. The docs also say that it doesn't hurt to call # CloseHandle on it. However, doing so from inside of Ruby always seems to produce an invalid handle error. - proc { close_handle(handle) unless handle == CURRENT_PROCESS_HANDLE } - end + # The recommendation is to use GetCurrentProcess instead of the const (HANDLE)-1, to ensure we're making the correct comparison. + return if handle == GetCurrentProcess() - def self.close_handle(handle) unless CloseHandle(handle) Chef::ReservedNames::Win32::Error.raise! end |