TTreeNode leak when VCL styles are active

[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

[balabuev 102]

@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;

 

 

 

[Attila Kovacs 631]

    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 March 9, 2021 by Attila Kovacs

[balabuev 102]

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 March 9, 2021 by balabuev

[Attila Kovacs 631]

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

it works

evt. could be further higher in the callstack

[Attila Kovacs 631]

stylehooks are no windowed controls like the scrollbar

[pyscripter 694]

1 hour ago, balabuev said:

 I can reproduce the bug even with this your workaround:

Could you tell us how?

[balabuev 102]

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.

 

 

[Attila Kovacs 631]

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.

[Attila Kovacs 631]

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 March 9, 2021 by Attila Kovacs

[balabuev 102]

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.

 

 

[Attila Kovacs 631]

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?

[balabuev 102]

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.

[Attila Kovacs 631]

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 March 9, 2021 by Attila Kovacs

[Attila Kovacs 631]

1 minute ago, balabuev said:

until the application end.

Aha! Now I'm getting closer.

Have to fire up a new form to check.

[pyscripter 694]

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.

[balabuev 102]

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.

 

 

[balabuev 102]

1 minute ago, pyscripter said:

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

Yes, exactly.

[balabuev 102]

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.

[Attila Kovacs 631]

Buhh. Well, this is a new problem.

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

 

Edited March 9, 2021 by Attila Kovacs

[balabuev 102]

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.

[Attila Kovacs 631]

I'm currently here

 

[Attila Kovacs 631]

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 March 9, 2021 by Attila Kovacs

[balabuev 102]

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

 

 

[pyscripter 694]

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 March 10, 2021 by pyscripter