opscode/mixlib-shellout — pull request #41 by adamedx (2014-03-17 22:26:55+00:00)
Title: MIXLIB-25: Only close handles that are set to a valid handle value

A user is hitting this problem in mixlib-shellout where in certain cases we try to call the Win32 method CloseHandle with nil -- the issue is that the wrapper for CloseHandle tries to access methods of nil and thus hits an exception.

The code is definitely incorrect, and the change here ensures that we only try to close valid handles (incidentally if we had used 0 things would have worked, but under a debugger Windows would throw an exception, and also, behavior is unclear to those not familair with CloseHandle).

This code path should not be possible though, so though the fix is correct (verified by the user), it's not clear why this flag exists in the hash. I'm trying to find more information from the user other than that the problem doesn't repro once the fix is deployed.

Another issue fixed is a handle leak -- the token handle in question is not freed in the case where you are running as a background service and try to run as an alternate user. I was able to repro the leak -- run code that creates 1000 processes using run_command and watch the handle count go over 1160 for the process -- with the fix, the handle count stays at the steady state for the process while the 1000 processes are run (about 160-180 handles).