Fix a bug causing crashes when the NtQueryObject worker thread times out

[maxbelanger]

We've observed that psutil can crash in a call to DeleteFiber when trying to clean up the NtQueryObject thread if it takes too long. Looking at the current implementation, it appears the introduction of ConvertThreadToFiber was intended to prevent memory leaks on Windows XP, where TerminateThread does not release the thread's initial stack (per MSDN).

The crash appears to be caused by a misuse of DeleteFiber, which (it appears) shouldn't be called on a converted thread's "main fiber", as releasing the thread normally is meant to take care of that particular cleanup.TerminateThread is seemingly responsible for releasing the thread's resources including the initial fiber state, so calling DeleteFiber can cause an access violation (equivalent to a double delete).

For a simple fix, this patch just removes the use of a fiber altogether, as psutil no longer supports Windows XP anyways and its use is no longer necessary.

While we're at it, this also fixes the signature of psutil_NtQueryObjectThread (casting to LPTHREAD_START_ROUTINE is considered bad).

[giampaolo]

Seems reasonable. Could you please also update HISTORY file describing the change? Thanks a lot.

On Fri, 3 Mar 2017 at 04:01, Max Bélanger ***@***.***> wrote: We've observed that psutil can crash in a call to DeleteFiber when trying to clean up the NtQueryObject thread if it takes too long. Looking at the current implementation, it appears the introduction of ConvertThreadToFiber was intended to prevent memory leaks on Windows XP, where TerminateThread does not release the thread's initial stack (per MSDN <https://msdn.microsoft.com/en-us/library/windows/desktop/ms686717(v=vs.85).aspx> ). The crash appears to be caused by a misuse of DeleteFiber, which (it appears) shouldn't be called on a converted thread's "main fiber", as releasing the thread normally is meant to take care of that particular cleanup.TerminateThread is seemingly responsible for releasing the thread's resources including the initial fiber state, so calling DeleteFiber can cause an access violation (equivalent to a double delete). For a simple fix, this patch just removes the use of a fiber altogether, as psutil no longer supports Windows XP anyways and its use is no longer necessary. ------------------------------ You can view, comment on, or merge this pull request online at: #984 Commit Summary - first pass - correct signature - remove fiber code, not needed as we don't support xp anymore File Changes - *M* psutil/arch/windows/process_handles.c <https://github.com/giampaolo/psutil/pull/984/files#diff-0> (16) - *M* psutil/arch/windows/process_handles.h <https://github.com/giampaolo/psutil/pull/984/files#diff-1> (2) Patch Links: - https://github.com/giampaolo/psutil/pull/984.patch - https://github.com/giampaolo/psutil/pull/984.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#984>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAplLOG8-EmfpaSmt0iM73_Bm-qgzWpVks5rhy4dgaJpZM4MRhjq> .

-- Giampaolo - http://grodola.blogspot.com