Jump to content
aehimself

TTreeNode leak when VCL styles are active

Recommended Posts

Guest
32 minutes ago, Gustav Schubert said:

SPOILER]

hi @Gustav Schubert you should use this format:  "["SPOILER]   <<-- beggin with [  --> else, the command is not valid, but just stay like a "common text".

 

hug

Share this post


Link to post

@pyscripter, forget about handles recreation in CM_STYLECHANGED. First, because it may happen by some other reason; recreation of handles is a part of VCL - nothing to do with that. Second, because this anyway does not fix the bug, since the user can change Z-order explicitly using ListView.BringToFront, for example.

 

I can reproduce the bug even with this your workaround:

procedure TListView.WndProc(var Message: TMessage);
begin
  if (Message.Msg = CM_STYLECHANGED) then
  begin
    if ViewStyle = vsReport then
      UpdateColumns;
  end
  else
    inherited;
end;

 

 

 

Share this post


Link to post
    if not (csDestroying in Control.ComponentState) then
    begin
      FControls.Items[Control].Free;
      FControls.Remove(Control);
    end;

in class procedure TStyleEngine.DoRemoveControl(Control: TWinControl);

cut nested calls to DestroyWindow 

 

Edited by Attila Kovacs

Share this post


Link to post
19 minutes ago, Attila Kovacs said:

cut nested calls to DestroyWindow 

Did you checked it somehow? I think, this will not change anything, because we still destroy control's style hook. Which will internally destroy its own parts, like scrollbar windows.

Edited by balabuev

Share this post


Link to post

just copy the vcl sources to your project dir and alter them

it works

evt. could be further higher in the callstack

Share this post


Link to post
1 hour ago, balabuev said:

 I can reproduce the bug even with this your workaround:

Could you tell us how?

Share this post


Link to post
1 hour ago, Attila Kovacs said:

if not (csDestroying in Control.ComponentState) then

You cannot use this condition, because this effectively leaves the corresponding style hook in the FControls dictionary forever. So, you'll have a leak of style hooks.

 

2 minutes ago, Attila Kovacs said:

stylehooks are no windowed controls like the scrollbar

Yes, but look at TScrollingStyleHook.Destroy method. It destroys FVertScrollWnd and FHorzScrollWnd owned objects, which are of TScrollWindow class.

 

 

Share this post


Link to post
6 minutes ago, balabuev said:

Yes, but look at TScrollingStyleHook.Destroy method. It destroys FVertScrollWnd and FHorzScrollWnd owned objects, which are of TScrollWindow class.

 

Exactly this is the problem. That is what I wanted to avoid. Those are parented to the parent of the ListView and will be destroyed by the window manager.

Share this post


Link to post

Ok, i see what you are saying, the remove should be evt. called, free() not. 

But the dictionary will be free'd even if it's not empty. So, I guess doesn't matter. 

 

Edit: no, calling remove results in other leaks. It was just fine that way.

Edited by Attila Kovacs

Share this post


Link to post
3 minutes ago, Attila Kovacs said:

Those are parented to the parent of the ListView and will be destroyed by the window manager

We have actually two parts in each TScrollWindow:

  • Real window represented by window handle
  • Delphi object of TScrollWindow class.

First one will be deleted by window manager, but the second - not. However, even the first one will be deleted only when the parent panel will be destroyed. Which is not acceptable as a good quality fix.

 

I'm actually have somewhat similar idea: we should work on a slightly lower layer. Let's style hooks be destroyed like they are destroyed now. But, TScrollingStyleHook.Destroy should not destroy its owned TScrollWindow objects immediatelly. Instead it should hide them (make non-visible) and collect in some global list for later asynchronous destrution.

 

 

Share this post


Link to post

The above non-quality? "fix" resolves the problem calling nested DestroyWindow and thus stopping WM_DESTROY propagation.

The application does not yield any new leaks so I don't understand your point, what am I missing?

Share this post


Link to post

When some ListView is destroyed, all associated resources, like style hooks and scrollbar windows should be also destroyed as soon as possible. Such stuff should not live until the application end.

Share this post


Link to post

in 

class destructor TStyleEngine.Destroy;

 

FreeAndNil(FControls);

 

will free all remaining StyleHooks.

 

Who cares by who or when will it be free'd on closing the application?

Edited by Attila Kovacs

Share this post


Link to post
1 minute ago, balabuev said:

until the application end.

Aha! Now I'm getting closer.

Have to fire up a new form to check.

Share this post


Link to post
11 minutes ago, balabuev said:

First one will be deleted by window manager, but the second - not.

This is not true.  StyleEngine.Destroys calls FreeControlHooks which destroys all remaining hooks.   However what @Attila Kovacs suggests is problematic, since you keep accumulating unreleased hooks if the application creates and destroys styled controls.

  • Like 1

Share this post


Link to post
1 minute ago, Attila Kovacs said:

Have to fire up a new form to check.

Not necessary. During the runtime of the application you can just create and destory as many ListViews dynamically as you want. I can create and destroy 1000 ListViews in a loop. All on the same parent panel.

 

Anyway, this is up to application. We cannot predict this.

 

 

Share this post


Link to post
1 minute ago, pyscripter said:

since you keep accumulating unreleased hooks if the application creates and destroys styled controls

Yes, exactly.

Share this post


Link to post
34 minutes ago, pyscripter said:

Could you tell us how?

I need some time to modify our demo to show this in one simple step. The problem is that we have currently four scrollbar windows, from which only two belong to list view (other two belong to tree view). This hides the bug, when they are not in the needed specific order relative to each other.

Share this post


Link to post

Buhh. Well, this is a new problem.

Still, there is no legitimate reason to call consecutive DestroyWindows.

 

Edited by Attila Kovacs

Share this post


Link to post

I think, csDestroying flag cannot be used also, because it is set in any situation:

  • Form as a whole is destroyed
  • Or, the control only is destroyed by the user.

Share this post


Link to post

this is really below every standard but what about 

 

  if not (csDestroying in Application.ComponentState) then

 

it works fine and I can't see any just as simple solution

Edited by Attila Kovacs

Share this post


Link to post
2 hours ago, pyscripter said:

Could you tell us how?

I've replaced the treeview with a frame to remove the mess with the second pair of unneeded scrollbar windows. I've simulated some resource allocation on WM_CREATE in a frame, which is then freed on WM_DESTROY, similar to tree view.

 

  • Run the demo.
  • Switch to any VCL Style.
  • Press "Bring to front" button.
  • Close the form

Source.zip

 

 

  • Like 1

Share this post


Link to post

How about this one?

 

class procedure TStyleEngine.DoRemoveControl(Control: TWinControl);
begin
  if not (csDestroying in Control.ComponentState) and (FControls <> nil) and FControls.ContainsKey(Control) then
  begin
    var Hook := FControls.Items[Control];
    TThread.ForceQueue(nil, procedure
    begin
      Hook.Free;
    end);
    FControls.Remove(Control);
  end;
end;

There may be more efficient solutions, such as adding the hooks for deletion to a List and process that list on idle time or on Hook creation.

 

Edited by pyscripter
  • Like 1

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×