Sign in Join

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

Mailing lists / lazarus / 2010-10

Memory leak in TThread class

17 messages · 7 people · –

Reply by email Subscribe to this list Your subscription

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

silvioprog

Hi folks,

I detected a small leak in the TThread class. I made a simple test and I sending in attached (or http://pastebin.com/aWNuHyDQ). To reproduce the leak, just open and close the demo.

My environment:

Lazarus 0.9.29 r27568 FPC 2.5.1 i386-win32-win32/win64

Silvio Cl?cio, *programmer* *ObjectPascal* -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20101004/b0b54d6c/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: ThreadLeak.zip Type: application/zip Size: 2017 bytes Desc: not available URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20101004/b0b54d6c/attachment.zip>

Reply by email

Marcos Douglas
to silvioprog

2010/10/4 silvioprog :

Show quoted text 6 lines
Hi folks, I detected a small leak in the TThread class. I made a simple test and I sending in attached (or?http://pastebin.com/aWNuHyDQ). To reproduce the leak, just open and close the demo. My environment: Lazarus 0.9.29 r27568 FPC 2.5.1 i386-win32-win32/win64

Where is the mem leak?

Marcos Douglas

Reply by email

silvioprog
to Marcos Douglas

2010/10/4 Marcos Douglas

Where is the mem leak?

------------------------------------------------------------------------------------ C:\Users\silvioprog\Desktop\ThreadLeak\project1.exe Heap dump by heaptrc unit
500 memory blocks allocated : 33396/34856
499 memory blocks freed : 33384/34840
1 unfreed memory blocks : 12
True heap size : 229376 (112 used in System startup) True free heap : 229168
Should be : 229184
Call trace for block $0018F270 size 12
$0040E5BB
$00439456
$0042E31C TFORM1__CREATE, line 41 of Unit1.pas
$0042CE98 TAPPLICATION__CREATEFORM, line 2084 of ./include/application.inc
$00402A83 main, line 17 of project1.lpr
$00410CC1
$00402A83 main, line 17 of project1.lpr
$00410CC1
------------------------------------------------------------------------------------

Marcos Douglas

Silvio Cl?cio, *programmer* *ObjectPascal* -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20101004/21e89f30/attachment.html>

Reply by email

Luca Olivetti
to silvioprog: I detected a small leak in the TThread class. I made a simple test and I sending in…

En/na silvioprog ha escrit:

Show quoted text 5 lines
Hi folks, I detected a small leak in the TThread class. I made a simple test and I sending in attached (or http://pastebin.com/aWNuHyDQ). To reproduce the leak, just open and close the demo.

That's probably because the thread has no chance to run (you create it suspended and never resume it).
If you create it with suspended=false or resume it right after creating it, there's no leak.

Bye

Luca Olivetti Wetron Automatizaci?n S.A. http://www.wetron.es/ Tel. +34 93 5883004 (Ext.133) Fax +34 93 5883007

Reply by email

Marcos Douglas
to Luca Olivetti
Luca Olivetti wrote 12 lines
En/na silvioprog ha escrit: > > Hi folks, > > I detected a small leak in the TThread class. I made a simple test and I > sending in attached (or http://pastebin.com/aWNuHyDQ). To reproduce the > leak, just open and close the demo. That's probably because the thread has no chance to run (you create it suspended and never resume it). If you create it with suspended=false or resume it right after creating it, there's no leak.

Even that, I consider this a bug, don't you?

Marcos Douglas

Reply by email

Luca Olivetti
to Marcos Douglas

En/na Marcos Douglas ha escrit:

Show quoted text 13 lines
On Tue, Oct 5, 2010 at 9:40 AM, Luca Olivetti wrote: > En/na silvioprog ha escrit: >> Hi folks, >> >> I detected a small leak in the TThread class. I made a simple test and I >> sending in attached (or http://pastebin.com/aWNuHyDQ). To reproduce the >> leak, just open and close the demo. > That's probably because the thread has no chance to run (you create it > suspended and never resume it). > If you create it with suspended=false or resume it right after creating it, > there's no leak. Even that, I consider this a bug, don't you?

I'm not sure it is: when I create a thread is because I need it to run, if it doesn't have to run I simply don't create it. In the constructor I usually delay starting the thread until all members are correctly initialized (i.e. "inherited create(true); foo:=Tfoo.create; resume"), but afterwards it has to run. If you forget to, say, free objects in a thread, it will cause a memory leak but it's not a bug, this case is analogous: you create a thread but forget to start it.

Bye

Luca Olivetti Wetron Automatizaci?n S.A. http://www.wetron.es/ Tel. +34 93 5883004 (Ext.133) Fax +34 93 5883007

Reply by email

Marcos Douglas
to Luca Olivetti
Luca Olivetti wrote 28 lines
En/na Marcos Douglas ha escrit: > > On Tue, Oct 5, 2010 at 9:40 AM, Luca Olivetti wrote: >> >> En/na silvioprog ha escrit: >>> >>> Hi folks, >>> >>> I detected a small leak in the TThread class. I made a simple test and I >>> sending in attached (or http://pastebin.com/aWNuHyDQ). To reproduce the >>> leak, just open and close the demo. >> >> That's probably because the thread has no chance to run (you create it >> suspended and never resume it). >> If you create it with suspended=false or resume it right after creating >> it, >> there's no leak. > > Even that, I consider this a bug, don't you? I'm not sure it is: when I create a thread is because I need it to run, if it doesn't have to run I simply don't create it. In the constructor I usually delay starting ?the thread until all members are correctly initialized (i.e. "inherited create(true); foo:=Tfoo.create; resume"), but afterwards it has to run. If you forget to, say, free objects in a thread, it will cause a memory leak but it's not a bug, this case is analogous: you create a thread but forget to start it.

IMHO, the act of creating and destroying it should not be related to running.

Marcos Douglas

Reply by email

waldo kitty
to Luca Olivetti: En/na silvioprog ha escrit: That's probably because the thread has no chance to run (you…
Luca Olivetti wrote 11 lines
En/na silvioprog ha escrit: > Hi folks, > > I detected a small leak in the TThread class. I made a simple test and I > sending in attached (or http://pastebin.com/aWNuHyDQ). To reproduce the leak, > just open and close the demo. That's probably because the thread has no chance to run (you create it suspended and never resume it). If you create it with suspended=false or resume it right after creating it, there's no leak.

if it has no chance to run, it should not allocate the memory...

if it allocates the memory during a create-suspended, it should un-allocate that memory during a destroy-suspended...

in other words, it should not matter if the thread is suspended or not... if it allocates memory during creation it should deallocate that memory during destruction...

Reply by email

Andrew Brunner
to waldo kitty
waldo kitty wrote 9 lines
if it has no chance to run, it should not allocate the memory... if it allocates the memory during a create-suspended, it should un-allocate that memory during a destroy-suspended... in other words, it should not matter if the thread is suspended or not... if it allocates memory during creation it should deallocate that memory during destruction...

This is what I feel *should* happen as well. Unfortunately, depending on OS and Version I think that is where the problem comes into play. I seem to recall that destroying a thread object in Delphi under Win98 or NT4 would generate kernel exceptions (ages ago). The kernel is responsible for deleting the actual thread object that an FPC thread object represents.

The problem probably lands with the notion that somewhere the FPC thread is calling ENDTHREAD or something of the like while the thread is in a created but suspended state (which may be entirely different than create and then suspend manually depending on OS).

Since FPC has their hands limited options with respect to threads that the operating system maintains then the only real solution that may prove useful is a work-a-round. To get past this issue in the past I have used a ResumeThread Call. The execute method codes to check the terminated so that was essentially bypassed inside itself before it loops and a call.

I RARELY use MyThread.Free FOR EXACTLY this reason. I typically set the FreeOnTerminate option to True and handle things in the blind.

So if coded properly, this issue can be avoided all-together. And I really don't think this is a bug per say, but a challenge that requires skill to avoid.

Reply by email

Tobias Giesen
to Andrew Brunner

Hi,

I think you shouldn't create suspended threads anyway. The Windows API docs say that suspend/resume is only for debuggers. To synchronize threads or to delay the starting of a thread you should use events, WaitForMultipleObjects etc.

Cheers,
Tobias

Reply by email

Sergei Gorelkin
to Tobias Giesen

Tobias Giesen ?????:

Show quoted text 7 lines
Hi, I think you shouldn't create suspended threads anyway. The Windows API docs say that suspend/resume is only for debuggers. To synchronize threads or to delay the starting of a thread you should use events, WaitForMultipleObjects etc.

Creating a suspended TThread is a different matter than suspending it later on. TThread has code to handle the initial suspend case, so there should be no need in additional user code. Further, to differentiate between 'initial' and 'subsequent' suspends, Delphi has introduced a method named 'Start'. It was added to FPC recently, too.

Regards,
Sergei

Reply by email

Marcos Douglas
to Sergei Gorelkin
Sergei Gorelkin wrote 15 lines
Tobias Giesen ?????: > > Hi, > > I think you shouldn't create suspended threads anyway. The Windows > API docs say that suspend/resume is only for debuggers. To synchronize > threads or to delay the starting of a thread you should use events, > WaitForMultipleObjects etc. > Creating a suspended TThread is a different matter than suspending it later on. TThread has code to handle the initial suspend case, so there should be no need in additional user code. Further, to differentiate between 'initial' and 'subsequent' suspends, Delphi has introduced a method named 'Start'. It was added to FPC recently, too.

Well, the problem was fixed.
Bug reporter: http://bugs.freepascal.org/view.php?id=16884 Solution: http://cia.vc/stats/project/fpc/.message/b8ef19

Marcos Douglas

Reply by email

Sergei Gorelkin
to Marcos Douglas
Marcos Douglas wrote

Unfortuately, not. Issue #16884 was a race condition which was causing the thread to start executing before it was completely initialized by its constructor. The current issue is a resource leak that happens when a TThread is created and then destroyed before executing any thread code.

Regards,
Sergei

Reply by email

Sergei Gorelkin
to Sergei Gorelkin
Sergei Gorelkin wrote 11 lines
Marcos Douglas wrote: > Well, the problem was fixed. > Bug reporter: http://bugs.freepascal.org/view.php?id=16884 > Solution: http://cia.vc/stats/project/fpc/.message/b8ef19 > Unfortuately, not. Issue #16884 was a race condition which was causing the thread to start executing before it was completely initialized by its constructor. The current issue is a resource leak that happens when a TThread is created and then destroyed before executing any thread code.

Reported as http://bugs.freepascal.org/view.php?id=17560

Regards,
Sergei

Reply by email

Marcos Douglas
to Sergei Gorelkin: Unfortuately, not. Issue #16884 was a race condition which was causing the thread to…
Sergei Gorelkin wrote 10 lines
Marcos Douglas wrote: > Well, the problem was fixed. > Bug reporter: http://bugs.freepascal.org/view.php?id=16884 > Solution: http://cia.vc/stats/project/fpc/.message/b8ef19 > Unfortuately, not. Issue #16884 was a race condition which was causing the thread to start executing before it was completely initialized by its constructor. The current issue is a resource leak that happens when a TThread is created and then destroyed before executing any thread code.

Oh man, sorry. My mistake. :(

Marcos Douglas

Reply by email

silvioprog
to Sergei Gorelkin: Reported as http://bugs.freepascal.org/view.php?id=17560 Sergei

Thanks to all. ;)

2010/10/6 Sergei Gorelkin

Show quoted text 18 lines
Sergei Gorelkin wrote: > Marcos Douglas wrote: > > Well, the problem was fixed. >> Bug reporter: http://bugs.freepascal.org/view.php?id=16884 >> Solution: http://cia.vc/stats/project/fpc/.message/b8ef19 >> >> Unfortuately, not. Issue #16884 was a race condition which was causing > the thread to start executing before it was completely initialized by its > constructor. The current issue is a resource leak that happens when a > TThread is created and then destroyed before executing any thread code. > > Reported as http://bugs.freepascal.org/view.php?id=17560 Regards, Sergei
Silvio Cl?cio, *programmer* *ObjectPascal* -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20101006/94ad2e8f/attachment.html>

Reply by email

Luca Olivetti
to Sergei Gorelkin: Unfortuately, not. Issue #16884 was a race condition which was causing the thread to…

En/na Sergei Gorelkin ha escrit:

Show quoted text 10 lines
Marcos Douglas wrote: > Well, the problem was fixed. > Bug reporter: http://bugs.freepascal.org/view.php?id=16884 > Solution: http://cia.vc/stats/project/fpc/.message/b8ef19 > Unfortuately, not. Issue #16884 was a race condition which was causing the thread to start executing before it was completely initialized by its constructor. The current issue is a resource leak that happens when a TThread is created and then destroyed before executing any thread code.

I don't see the bug there: as soon as you call "inherited create(false)" you know that the thread can start executing, that's the point of creating it *not* suspended.
If you want to be sure that the constructor has initialized everything, either call "inherited create(false)" as the last instruction of the constructor or use "inherited create(true)" and later call "resume". Oh, well, maybe I'm just used to defensive programming.

Bye

Luca Olivetti Wetron Automatizaci?n S.A. http://www.wetron.es/ Tel. +34 93 5883004 (Ext.133) Fax +34 93 5883007

Reply by email