Sign in Join

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

Mailing lists / lazarus / 2003-11

CodeExplorer memory leak

5 messages · 3 people ·

Reply by email Subscribe to this list Your subscription

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

Mattias Gaertner
to Vincent Snijders: I noticed the code explorer treeview has a memory leak. A TViewNodeData object is added…
Show quoted text 46 lines
On Sat, 29 Nov 2003 11:10:17 +0100 Marc Weustink wrote: > At 10:56 29-11-2003, Vincent Snijders wrote: > >Hi, > > > >I noticed the code explorer treeview has a memory leak. > > > >A TViewNodeData object is added to each tree node and stored in the > >Data property. When the tree node is freeed by TreeView.Items.Clear > >these TViewNodeData objects are not freeed. > > This is a known issue if you use Delphi :) > > >I could think of several solutions to fix this and want your input as > >to which you want. > > > >1. Change TTreeNode.Delete, so that it not only set Data to nil, but > >also does FData.Free if FData is an TObject. IMO this is dangerous, > >because the TreeNode might not be the owner of FData. > > If so this sould be configurable. And what is Data is not an object > but some memory to be freed. (it contains a record allocated by new) > Not the best solution > > >2. Add an OnDestroy Event to TTreeNode, which the CodeExplorer can > >use to free the TViewNodeData objects. IMO, this is the best > >solution. > > Or better add an OnNodeDestroy to the TTree, then you dont have to set > > OnDestroy on each node. > This is indeed the most generic solution. > > >3. Before calling TreeView.Items.Clear in the code explorer, free and > >set to nil the data objects. Also possible, if you don't want to > >extend the LCL. > > This is the common sulution in Delphi as well. > > >4. Any other way, I couldn't think of. > > See 2 > Further code inspection showed, that most of it was already present. The treeview has an OnDeletion event. It just wasn't called in every case.

:)

Attached patch fixes this.

Applied.

Show quoted text 6 lines
I hope I didn't break any treeview existing code. If so, let me know. I also noticed there is a lot of commented code in treeview.inc. Don't you think much of it can be deleted? Will it ever be needed in the future.

The comments with the ItemIDs can be deleted. The parts showing the undocumented logic behind Delphi's TTreeView D&D should be kept.

Mattias

Reply by email

Vincent Snijders
to Vincent Snijders: I noticed the code explorer treeview has a memory leak. A TViewNodeData object is added…
Show quoted text 41 lines
At 10:56 29-11-2003, Vincent Snijders wrote: >Hi, > >I noticed the code explorer treeview has a memory leak. > >A TViewNodeData object is added to each tree node and stored in the >Data property. When the tree node is freeed by TreeView.Items.Clear >these TViewNodeData objects are not freeed. This is a known issue if you use Delphi :) >I could think of several solutions to fix this and want your input as >to which you want. > >1. Change TTreeNode.Delete, so that it not only set Data to nil, but >also does FData.Free if FData is an TObject. IMO this is dangerous, >because the TreeNode might not be the owner of FData. If so this sould be configurable. And what is Data is not an object but some memory to be freed. (it contains a record allocated by new) Not the best solution >2. Add an OnDestroy Event to TTreeNode, which the CodeExplorer can >use to free the TViewNodeData objects. IMO, this is the best >solution. Or better add an OnNodeDestroy to the TTree, then you dont have to set OnDestroy on each node. This is indeed the most generic solution. >3. Before calling TreeView.Items.Clear in the code explorer, free and >set to nil the data objects. Also possible, if you don't want to >extend the LCL. This is the common sulution in Delphi as well. >4. Any other way, I couldn't think of. See 2

Further code inspection showed, that most of it was already present. The treeview has an OnDeletion event. It just wasn't called in every case. Attached patch fixes this. I hope I didn't break any treeview existing code. If so, let me know.

I also noticed there is a lot of commented code in treeview.inc. Don't you think much of it can be deleted? Will it ever be needed in the future.

Regards,
Vincent.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: gz00048.gz
Type: application/octet-stream
Size: 456 bytes
Desc: "treeview.patch.gz"
Url : http://localhost/pipermail/lazarus/attachments/20031129/5f8e58cc/gz00048.obj

Reply by email

Vincent Snijders
to Vincent Snijders: I noticed the code explorer treeview has a memory leak. A TViewNodeData object is added…
Show quoted text 40 lines
At 10:56 29-11-2003, Vincent Snijders wrote: >Hi, > >I noticed the code explorer treeview has a memory leak. > >A TViewNodeData object is added to each tree node and stored in the >Data property. When the tree node is freeed by TreeView.Items.Clear >these TViewNodeData objects are not freeed. This is a known issue if you use Delphi :) >I could think of several solutions to fix this and want your input as >to which you want. > >1. Change TTreeNode.Delete, so that it not only set Data to nil, but >also does FData.Free if FData is an TObject. IMO this is dangerous, >because the TreeNode might not be the owner of FData. If so this sould be configurable. And what is Data is not an object but some memory to be freed. (it contains a record allocated by new) Not the best solution >2. Add an OnDestroy Event to TTreeNode, which the CodeExplorer can >use to free the TViewNodeData objects. IMO, this is the best >solution. Or better add an OnNodeDestroy to the TTree, then you dont have to set OnDestroy on each node. This is indeed the most generic solution. >3. Before calling TreeView.Items.Clear in the code explorer, free and >set to nil the data objects. Also possible, if you don't want to >extend the LCL. This is the common sulution in Delphi as well. >4. Any other way, I couldn't think of. See 2

I will do so.

Vincent.

Reply by email

Marc Weustink
to Vincent Snijders: I noticed the code explorer treeview has a memory leak. A TViewNodeData object is added…
Vincent Snijders wrote 7 lines
Hi, I noticed the code explorer treeview has a memory leak. A TViewNodeData object is added to each tree node and stored in the Data property. When the tree node is freeed by TreeView.Items.Clear these TViewNodeData objects are not freeed.

This is a known issue if you use Delphi :)

Show quoted text 6 lines
I could think of several solutions to fix this and want your input as to which you want. 1. Change TTreeNode.Delete, so that it not only set Data to nil, but also does FData.Free if FData is an TObject. IMO this is dangerous, because the TreeNode might not be the owner of FData.

If so this sould be configurable. And what is Data is not an object but some memory to be freed. (it contains a record allocated by new) Not the best solution

2. Add an OnDestroy Event to TTreeNode, which the CodeExplorer can use to free the TViewNodeData objects. IMO, this is the best solution.

Or better add an OnNodeDestroy to the TTree, then you dont have to set OnDestroy on each node.
This is indeed the most generic solution.

3. Before calling TreeView.Items.Clear in the code explorer, free and set to nil the data objects. Also possible, if you don't want to extend the LCL.

This is the common sulution in Delphi as well.

4. Any other way, I couldn't think of.

See 2

In what way, do you want me to supply a patch? (If you say 4., please specify.)

Marc

Reply by email

Vincent Snijders

Hi,

I noticed the code explorer treeview has a memory leak.

A TViewNodeData object is added to each tree node and stored in the Data property. When the tree node is freeed by TreeView.Items.Clear these TViewNodeData objects are not freeed.

I could think of several solutions to fix this and want your input as to which you want.

1. Change TTreeNode.Delete, so that it not only set Data to nil, but also does FData.Free if FData is an TObject. IMO this is dangerous, because the TreeNode might not be the owner of FData.

2. Add an OnDestroy Event to TTreeNode, which the CodeExplorer can use to free the TViewNodeData objects. IMO, this is the best solution.

3. Before calling TreeView.Items.Clear in the code explorer, free and set to nil the data objects. Also possible, if you don't want to extend the LCL.
4. Any other way, I couldn't think of.

In what way, do you want me to supply a patch? (If you say 4., please specify.)

Regards,
Vincent.

Reply by email