From ffc357a528d5a9e90de137327278069219fbaead Mon Sep 17 00:00:00 2001 From: sergei Date: Mon, 1 Nov 2010 22:37:33 +0000 Subject: [PATCH] * TThread, Windows implementation: prevent resource leak when destroying an initially suspended and never resumed thread. The thread must be always resumed so that ThreadProc can complete and cleanup. Fixes Mantis #17560. git-svn-id: trunk@16290 - --- rtl/objpas/classes/classes.inc | 6 +++++- rtl/win/tthread.inc | 9 ++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/rtl/objpas/classes/classes.inc b/rtl/objpas/classes/classes.inc index 2f37ecbc3b..75a9c4acb8 100644 --- a/rtl/objpas/classes/classes.inc +++ b/rtl/objpas/classes/classes.inc @@ -90,7 +90,11 @@ function ThreadProc(ThreadObjPtr: Pointer): PtrInt; // will call the AfterConstruction method in all cases) // Thread.Suspend; try - Thread.Execute; + { The thread may be already terminated at this point, e.g. if it was intially + suspended, or if it wasn't ever scheduled for execution for whatever reason. + So bypass user code if terminated. } + if not Thread.Terminated then + Thread.Execute; except Thread.FFatalException := TObject(AcquireExceptionObject); end; diff --git a/rtl/win/tthread.inc b/rtl/win/tthread.inc index 7d04e0b2bd..aa6203d1e5 100644 --- a/rtl/win/tthread.inc +++ b/rtl/win/tthread.inc @@ -30,9 +30,16 @@ destructor TThread.Destroy; begin if FHandle<>0 then begin - if not FFinished and not Suspended then + { Don't check Suspended. If the thread has been externally suspended (which is + deprecated and strongly discouraged), it's better to deadlock here than + to silently free the object and leave OS resources leaked. } + if not FFinished {and not Suspended} then begin Terminate; + { Allow the thread function to perform the necessary cleanup. Since + we've just set Terminated flag, it won't call Execute. } + if FInitialSuspended then + Start; WaitFor; end; CloseHandle(FHandle);