changes in TThread.OnTerminate in fcp-3.0?

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

changes in TThread.OnTerminate in fcp-3.0?

Luca Olivetti-2
Hello,

in a couple of programs I'm using this idiom:


FMyThread:=TMyTread.Create;
FMyThread.FreeOnTerminate:=false;
FMyThread.OnTerminate:=@MyThreadTerminate

procedure TMyForm.MyThreadTerminate(Sender:TObject);
begin
   //do something with FMyThread then
   FreeAndNil(FMyThread);
end;


With fpc 2.6.4 it worked flawlessly, with fpc 3.0 sometimes it causes a
sigsev somewhere inside CheckSynchronize.

I could find nothing here
http://wiki.freepascal.org/User_Changes_3.0
related to this.
I realize that's an implementation detail so probably what I did was
wrong (was it?).
What are the possible solutions (apart from omitting the call to
FreeAndNil and setting FreeOnTerminate to true)?


Bye
--
Luca
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Sven Barth-2

Am 10.12.2015 11:05 schrieb "Luca Olivetti" <[hidden email]>:
>
> Hello,
>
> in a couple of programs I'm using this idiom:
>
>
> FMyThread:=TMyTread.Create;
> FMyThread.FreeOnTerminate:=false;
> FMyThread.OnTerminate:=@MyThreadTerminate
>
> procedure TMyForm.MyThreadTerminate(Sender:TObject);
> begin
>   //do something with FMyThread then
>   FreeAndNil(FMyThread);
> end;
>
>
> With fpc 2.6.4 it worked flawlessly, with fpc 3.0 sometimes it causes a sigsev somewhere inside CheckSynchronize.
>
> I could find nothing here
> http://wiki.freepascal.org/User_Changes_3.0
> related to this.
> I realize that's an implementation detail so probably what I did was wrong (was it?).

I'd need to check what the RTL does after OnTerminate is called, but it could be that this worked by chance...

> What are the possible solutions (apart from omitting the call to FreeAndNil and setting FreeOnTerminate to true)?

That would be the safest solution (you can set FMyThread to Nil however inside that handler).

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Henry Vermaak
In reply to this post by Luca Olivetti-2
On Thu, Dec 10, 2015 at 10:26:29AM +0100, Luca Olivetti wrote:

> Hello,
>
> in a couple of programs I'm using this idiom:
>
>
> FMyThread:=TMyTread.Create;
> FMyThread.FreeOnTerminate:=false;
> FMyThread.OnTerminate:=@MyThreadTerminate
>
> procedure TMyForm.MyThreadTerminate(Sender:TObject);
> begin
>   //do something with FMyThread then
>   FreeAndNil(FMyThread);
> end;

This is why FreeOnTerminate exists.  Set that to True and set FMyThread
to nil here, and the thread will free itself after this function
returns.

Henry
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Luca Olivetti-2
In reply to this post by Sven Barth-2
El 10/12/15 a les 11:13, Sven Barth ha escrit:

> Am 10.12.2015 11:05 schrieb "Luca Olivetti" <[hidden email]
> <mailto:[hidden email]>>:
>  >
>  > Hello,
>  >
>  > in a couple of programs I'm using this idiom:
>  >
>  >
>  > FMyThread:=TMyTread.Create;
>  > FMyThread.FreeOnTerminate:=false;
>  > FMyThread.OnTerminate:=@MyThreadTerminate
>  >
>  > procedure TMyForm.MyThreadTerminate(Sender:TObject);
>  > begin
>  >   //do something with FMyThread then
>  >   FreeAndNil(FMyThread);
>  > end;
>  >
>  >
>  > With fpc 2.6.4 it worked flawlessly, with fpc 3.0 sometimes it causes
> a sigsev somewhere inside CheckSynchronize.
>  >
>  > I could find nothing here
>  > http://wiki.freepascal.org/User_Changes_3.0
>  > related to this.
>  > I realize that's an implementation detail so probably what I did was
> wrong (was it?).
>
> I'd need to check what the RTL does after OnTerminate is called, but it
> could be that this worked by chance...

Yes and no: OnTerminate is called via Synchronize. In 2.6.4 there could
be only one outstanding synchronized method and CheckSynchronize
wouldn't access the thread after calling it, so freeing the thread
inside OnTerminate was no problem.

Now it's been changed to use a queue of waiting methods, and
CheckSynchronize, after calling the method, calls

    RtlEnventSetEvent(tmpentry^.SyncEvent)

where SyncEvent is a member of the thread, and if the thread has already
been freed that's going to bomb.

It's obvious that you cannot free the thread from inside a "normal"
Synchronize call, it's no so obvious you cannot do it in the OnTerminate
method, so maybe a note could be added in

http://wiki.freepascal.org/User_Changes_3.0

The new implementation is clearly better....once you know *not* to free
the thread inside OnTerminate.


Bye
--
Luca
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Anthony Walter-3
I use a four method pattern with threads, while reusing the same thread class everywhere.

1) define a status method to update your user interface based on feedback from the thread
2) define a terminate method to do clean up when a thread finishes
3) define a work method where the thread does its execution
4) define a method which starts the thread feeding it all three methods above

procedure TReplaceForm.ThreadStatus(Sender: TObject);
begin
  Caption := FThread.Status;
end;

procedure TReplaceForm.ThreadTerminate(Sender: TObject);
begin
  FThread := nil;
end;

procedure TReplaceForm.ThreadExecute(Thread: TSimpleThread);
begin
  Thread.Status := 'Step 1';
  Sleep(1000);
  Thread.Status := 'Step 2';
  Sleep(1000);
end;

procedure TReplaceForm.ReplaceButtonClick(Sender: TObject);
begin
  if FThread = nil then
    FThread := TSimpleThread.Create(ThreadExecute, ThreadStatus, ThreadTerminate);
end;
And done. You never have to define your own TThread derived class again.


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Sven Barth-2
In reply to this post by Luca Olivetti-2

Am 10.12.2015 12:15 schrieb "Luca Olivetti" <[hidden email]>:
> It's obvious that you cannot free the thread from inside a "normal" Synchronize call, it's no so obvious you cannot do it in the OnTerminate method, so maybe a note could be added in
>
> http://wiki.freepascal.org/User_Changes_3.0
>
> The new implementation is clearly better....once you know *not* to free the thread inside OnTerminate.

In all honesty it was never documented either that you *can* free the object from within OnTerminate and in other occurrences you wouldn't free the object that invoked the handler either, right? So why for TThread then?
=> no need to mention as a change

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Henry Vermaak
On Thu, Dec 10, 2015 at 02:49:50PM +0100, Sven Barth wrote:

> Am 10.12.2015 12:15 schrieb "Luca Olivetti" <[hidden email]>:
> > It's obvious that you cannot free the thread from inside a "normal"
> Synchronize call, it's no so obvious you cannot do it in the
> OnTerminate method, so maybe a note could be added in
> >
> > http://wiki.freepascal.org/User_Changes_3.0
> >
> > The new implementation is clearly better....once you know *not* to
> > free
> the thread inside OnTerminate.
>
> In all honesty it was never documented either that you *can* free the
> object from within OnTerminate and in other occurrences you wouldn't
> free the object that invoked the handler either, right? So why for
> TThread then?  => no need to mention as a change

Is it worthwhile mentioning that OnTerminate gets called with
Synchronize()?  I had to dig into the source to verify this.

Henry
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Sven Barth-2

Am 10.12.2015 15:06 schrieb "Henry Vermaak" <[hidden email]>:
>
> On Thu, Dec 10, 2015 at 02:49:50PM +0100, Sven Barth wrote:
> > Am 10.12.2015 12:15 schrieb "Luca Olivetti" <[hidden email]>:
> > > It's obvious that you cannot free the thread from inside a "normal"
> > Synchronize call, it's no so obvious you cannot do it in the
> > OnTerminate method, so maybe a note could be added in
> > >
> > > http://wiki.freepascal.org/User_Changes_3.0
> > >
> > > The new implementation is clearly better....once you know *not* to
> > > free
> > the thread inside OnTerminate.
> >
> > In all honesty it was never documented either that you *can* free the
> > object from within OnTerminate and in other occurrences you wouldn't
> > free the object that invoked the handler either, right? So why for
> > TThread then?  => no need to mention as a change
>
> Is it worthwhile mentioning that OnTerminate gets called with
> Synchronize()?  I had to dig into the source to verify this.

That would indeed be worth mentioning, but that would belong in the documentation then. Would you file a bug report, please?

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: changes in TThread.OnTerminate in fcp-3.0?

Henry Vermaak
On Thu, Dec 10, 2015 at 05:23:41PM +0100, Sven Barth wrote:
> Am 10.12.2015 15:06 schrieb "Henry Vermaak" <[hidden email]>:
> > Is it worthwhile mentioning that OnTerminate gets called with
> > Synchronize()?  I had to dig into the source to verify this.
>
> That would indeed be worth mentioning, but that would belong in the
> documentation then. Would you file a bug report, please?

Sure: http://bugs.freepascal.org/view.php?id=29166

Henry
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal