Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2018-06

TThread.Queue and TThread.Destroy

4 messages · 2 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Martin

fpc 3.0.4 / Linux 64bit (Fedora)

What should happen if:

- A Thread has queued a call with "TThread.Queue"
- The Thread gets Terminated and Destroyed before the queued method can be executed.
  That is the thread checks for "Terminated" and exits Execute before the queued method can be executed.

Diving into TThread.Destroy I can see: RemoveQueuedEvents(Self); So I would assume that all the queued calls are removed, and will not be executed.

I have code where the object to which the queued methods belong is destroyed, after TThread.Destroy. It works in about 80% of the cases (not enough).
I have put lots of debugln (write to console) in my code. And according to them one of the queued methods gets called, after the TThread.Destroy.

It definetely gets called from the queue, I caught it in gdb: my method
#0  0x000000000139b34a in HANDLEPIPEINPUT (this=0x7fffc9f0c4c0, ADATA=0, AREASONS=1) at debugprocess.pas:241
#1  0x000000000139b1ab in THREADDATAAVAIL (this=0x7fffc9f0c4c0) at debugprocess.pas:220
comming from the queue
#2  0x000000000054af14 in EXECUTETHREADQUEUEENTRY (AENTRY=0x7fffc9f0c4c0) at ../objpas/classes/classes.inc:272 #3  0x0000000002486c10 in  ()
#4  0x000000000054b333 in CHECKSYNCHRONIZE (TIMEOUT=0) at ../objpas/classes/classes.inc:404
#5  0x000000000050c67b in THREADSYNC_IOCALLBACK (SOURCE=0x255abf0, CONDITION=1, DATA=0x7ffff7f84470)
    at gtk2/gtk2widgetset.inc:1843
#6  0x00007ffff65db7cd in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#7  0x00007ffff65dbb98 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0
#8  0x00007ffff65dbc30 in g_main_context_iteration () at /lib64/libglib-2.0.so.0
#9  0x000000000050d920 in APPWAITMESSAGE (this=0x7ffff7f84470) at gtk2/gtk2widgetset.inc:2431
#10 0x000000000048de9f in IDLE (this=0x7ffff7f83d10, WAIT=true) at include/application.inc:414

Logging shows, that the process was destroyed before, and had left the thread's "Execute" method.

Any ideas.

Some more info. The Thread queued 2 events (actually it queued the 2nd event, while the main thread was processing the first event). The 2nd event is executed as soon as the first one finished.

--------------------------
Possibly I found what happens:
in classes.inc

class procedure TThread.RemoveQueuedEvents(aMethod: TThreadMethod);
begin
   RemoveQueuedEvents(Nil, aMethod);
end;

which I believe is called from TThread.Destroy.

But then
class procedure TThread.RemoveQueuedEvents(aThread: TThread; aMethod:
TThreadMethod);
var
   entry, tmpentry, lastentry: PThreadQueueEntry;
begin
   { anything to do at all? }
   if not Assigned(aThread) or not Assigned(aMethod) then
     Exit;

this just exits, if aMethod is nil.

So the events are not removed at all?

Reply by email

Martin
to Martin
Martin wrote 13 lines
fpc 3.0.4 / Linux 64bit (Fedora) What should happen if: - A Thread has queued a call with "TThread.Queue" - The Thread gets Terminated and Destroyed before the queued method can be executed.   That is the thread checks for "Terminated" and exits Execute before the queued method can be executed. Diving into TThread.Destroy I can see: RemoveQueuedEvents(Self); So I would assume that all the queued calls are removed, and will not be executed.

...

Show quoted text 9 lines
class procedure TThread.RemoveQueuedEvents(aThread: TThread; aMethod: TThreadMethod); var   entry, tmpentry, lastentry: PThreadQueueEntry; begin   { anything to do at all? }   if not Assigned(aThread) or not Assigned(aMethod) then     Exit;

Arrgh, my fault, the above is correct, but instead a few lines further

       if Assigned(aThread) and (entry^.Thread  aThread) then begin
         lastentry := entry;
         entry := entry^.Next;
         Continue;
       end;
       { then check for the method }
       if entry^.Method  aMethod then begin  ///////////// <<< should
that not also be "if assigned(aMethod) and ..." // entry^.Method will
never be equal to nil
         lastentry := entry;
         entry := entry^.Next;
         Continue;
       end;

But even then, I made a test project and still get the call. tested with fpc 3.0.4 on
- win 10 64 / but 32 bit fpc
- fedora 64bit

program Project1;

uses
  // cthreads,  // uncomment on Linux
  Classes, sysutils;

type
   TTestThread = class(TThread)
   protected
     procedure Execute; override;
   public
   end;
   TTest = class
   public
     procedure CallMeNot;
   end;
var
   Foo: TTestThread;
   Test: TTest;
   event: PRTLEvent;
procedure TTestThread.Execute;
begin
   Queue(@Test.CallMeNot);
   RTLeventSetEvent(event);
   while not Terminated do ;
end;
procedure TTest.CallMeNot;
begin
   writeln('shouldnt be here');
end;
begin
   event := RTLEventCreate;
   Test := TTest.Create;
   Foo := TTestThread.Create(False);
   RTLeventWaitFor(event); // make sure the event is queued
   //TThread.RemoveQueuedEvents(foo, @Test.CallMeNot); // this works
   Sleep(500);
   Foo.Terminate;
   foo.Destroy;
   TThread.RemoveQueuedEvents(foo, @Test.CallMeNot); // this does not
remove it in most cases // foo still has the value of the pointer to
freed mem, so it should
   sleep(500);
   CheckSynchronize();
   writeln('done');
   readln;
end.

Reply by email

Sven Barth
to Martin
Martin wrote 39 lines
On 21/06/2018 01:27, Martin wrote: > fpc 3.0.4 / Linux 64bit (Fedora) > > What should happen if: > > - A Thread has queued a call with "TThread.Queue" > - The Thread gets Terminated and Destroyed before the queued method > can be executed. >   That is the thread checks for "Terminated" and exits Execute before > the queued method can be executed. > > Diving into TThread.Destroy I can see: RemoveQueuedEvents(Self); > So I would assume that all the queued calls are removed, and will not > be executed. ... > > class procedure TThread.RemoveQueuedEvents(aThread: TThread; aMethod: > TThreadMethod); > var >   entry, tmpentry, lastentry: PThreadQueueEntry; > begin >   { anything to do at all? } >   if not Assigned(aThread) or not Assigned(aMethod) then >     Exit; Arrgh, my fault, the above is correct, but instead a few lines further       if Assigned(aThread) and (entry^.Thread aThread) then begin         lastentry := entry;         entry := entry^.Next;         Continue;       end;       { then check for the method }       if entry^.Method aMethod then begin  ///////////// <<< should that not also be "if assigned(aMethod) and ..." // entry^.Method will never be equal to nil         lastentry := entry;         entry := entry^.Next;         Continue;       end;

Would you please test with trunk? There it already checks for aMethod.

Also at least on x86_64-win64 your example works correctly, though that you're calling foo.Destroy directly after foo.Terminate is potentially dangerous: depending on the current system load the thread instance might be freed before the thread has fully finished executing (there is a bit of cleanup stuff happening after the thread returns from Execute). So the safest is to call foo.WaitFor before calling foo.Destroy and in that case WaitFor will call CheckSynchronize until the thread really finished (at least on Windows, OS/2 and Unix-like) as OnTerminate (if assigned) is called using Synchronize.

Regards,
Sven

Reply by email