Discussion:
TThread.Queue and TThread.Destroy
(too old to reply)
Martin
2018-06-20 23:27:43 UTC
Permalink
Raw Message
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?


_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman
Martin
2018-06-21 08:26:42 UTC
Permalink
Raw Message
Post by Martin
fpc 3.0.4 / Linux 64bit (Fedora)
- 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.
...
Post by Martin
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.

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-d
Sven Barth via fpc-devel
2018-06-21 19:21:50 UTC
Permalink
Raw Message
Post by Martin
Post by Martin
fpc 3.0.4 / Linux 64bit (Fedora)
- 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.
...
Post by Martin
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
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepas
Martin
2018-06-22 16:56:57 UTC
Permalink
Raw Message
Post by Sven Barth via fpc-devel
Would you please test with trunk? There it already checks for aMethod.
Indeed, it works with trunk.


_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.

Loading...