Browse Source

* return thread function result via pthread_exit() from CBeginThread
(Vinzent Hoefler)
* simplified CWaitForThreadTerminate based on comments from Vinzent
Hoefler
* fixed resource leaks where in some cases a pthread would not be
reaped based on comments from Vinzent Hoefler (resolves #9016)

git-svn-id: trunk@7588 -

Jonas Maebe 18 years ago
parent
commit
4dd3be0e5a
2 changed files with 30 additions and 16 deletions
  1. 2 5
      rtl/unix/cthreads.pp
  2. 28 11
      rtl/unix/tthread.inc

+ 2 - 5
rtl/unix/cthreads.pp

@@ -208,7 +208,7 @@ Type  PINTRTLEvent = ^TINTRTLEvent;
 {$endif DEBUG_MT}
 {$endif DEBUG_MT}
         ThreadMain:=pointer(ti.f(ti.p));
         ThreadMain:=pointer(ti.f(ti.p));
         DoneThread;
         DoneThread;
-        pthread_exit(nil);
+        pthread_exit(ThreadMain);
       end;
       end;
 
 
 
 
@@ -314,12 +314,9 @@ Type  PINTRTLEvent = ^TINTRTLEvent;
   function  CWaitForThreadTerminate (threadHandle : TThreadID; TimeoutMs : longint) : dword;  {0=no timeout}
   function  CWaitForThreadTerminate (threadHandle : TThreadID; TimeoutMs : longint) : dword;  {0=no timeout}
     var
     var
       LResultP: Pointer;
       LResultP: Pointer;
-      LResult: DWord;
     begin
     begin
-      LResult := 0;
-      LResultP := @LResult;
       pthread_join(pthread_t(threadHandle), @LResultP);
       pthread_join(pthread_t(threadHandle), @LResultP);
-      CWaitForThreadTerminate := LResult;
+      CWaitForThreadTerminate := dword(LResultP);
     end;
     end;
 
 
 {$warning threadhandle can be larger than a dword}
 {$warning threadhandle can be larger than a dword}

+ 28 - 11
rtl/unix/tthread.inc

@@ -121,9 +121,11 @@ begin
       WRITE_DEBUG('Thread ',ptrint(lthread),' should be freed');
       WRITE_DEBUG('Thread ',ptrint(lthread),' should be freed');
       LThread.Free;
       LThread.Free;
       WRITE_DEBUG('Thread freed');
       WRITE_DEBUG('Thread freed');
-//    tthread.destroy already frees all things and terminates the thread
-//    WRITE_DEBUG('thread func calling EndThread');
-//    EndThread(Result);
+      WRITE_DEBUG('thread func calling EndThread');
+      // we can never come here if the thread has already been joined, because
+      // this function is the thread's main function (so it would have terminated
+      // already in case it was joined)
+      EndThread(Result);
     end
     end
   else
   else
     begin
     begin
@@ -144,6 +146,7 @@ begin
     raise EThread.create('Semaphore init failed (possibly too many concurrent threads)');
     raise EThread.create('Semaphore init failed (possibly too many concurrent threads)');
   FSuspended := CreateSuspended;
   FSuspended := CreateSuspended;
   FSuspendedExternal := false;
   FSuspendedExternal := false;
+  FThreadReaped := false;
   FInitialSuspended := CreateSuspended;
   FInitialSuspended := CreateSuspended;
   FFatalException := nil;
   FFatalException := nil;
   WRITE_DEBUG('creating thread, self = ',longint(self));
   WRITE_DEBUG('creating thread, self = ',longint(self));
@@ -169,22 +172,33 @@ begin
       inherited destroy;
       inherited destroy;
       exit;
       exit;
     end;
     end;
-  if (FThreadID = GetCurrentThreadID) and not(FFreeOnTerminate) and not FFinished then
-    raise EThreadDestroyCalled.Create('A thread cannot destroy itself except by setting FreeOnTerminate and leaving!');
   // if someone calls .Free on a thread with
   // if someone calls .Free on a thread with
   // FreeOnTerminate, then don't crash!
   // FreeOnTerminate, then don't crash!
   FFreeOnTerminate := false;
   FFreeOnTerminate := false;
-  if not FFinished then
+  if (FThreadID = GetCurrentThreadID) then
     begin
     begin
-      Terminate;
-      if (FInitialSuspended) then
-        Resume;
-      WaitFor;
+      if not(FFreeOnTerminate) and not FFinished then
+        raise EThreadDestroyCalled.Create('A thread cannot destroy itself except by setting FreeOnTerminate and leaving!');
+      FFreeOnTerminate := false;
+    end
+  else
+    begin
+      { you can't join yourself, so only for FThreadID<>GetCurrentThreadID }
+      { and you can't join twice -> make sure we didn't join already       }
+      if not FThreadReaped then
+        begin
+          Terminate;
+          if (FInitialSuspended) then
+            Resume;
+          WaitFor;
+        end;
     end;
     end;
   CurrentTM.SemaphoreDestroy(FSem);
   CurrentTM.SemaphoreDestroy(FSem);
   FFatalException.Free;
   FFatalException.Free;
   FFatalException := nil;
   FFatalException := nil;
-  { threadvars have been released by cthreads.ThreadMain -> DoneThread }
+  { threadvars have been released by cthreads.ThreadMain -> DoneThread, or  }
+  { or will be released (in case of FFreeOnTerminate) after this destructor }
+  { has exited by ThreadFunc->EndThread->cthreads.CEndThread->DoneThread)   }
   inherited Destroy;
   inherited Destroy;
 end;
 end;
 
 
@@ -243,6 +257,9 @@ function TThread.WaitFor: Integer;
 begin
 begin
   WRITE_DEBUG('waiting for thread ',ptrint(FHandle));
   WRITE_DEBUG('waiting for thread ',ptrint(FHandle));
   WaitFor := WaitForThreadTerminate(FHandle, 0);
   WaitFor := WaitForThreadTerminate(FHandle, 0);
+  { should actually check for errors in WaitForThreadTerminate, but no }
+  { error api is defined for that function                             }
+  FThreadReaped:=true;
   WRITE_DEBUG('thread terminated');
   WRITE_DEBUG('thread terminated');
 end;
 end;