I have a fix for LzSprite that I will send out in the am. There are two bugs: the splitting of view into sprite got people confused about what the recursive flag means, and also lost the magic that keeps events on deleted objects from being fired (thus sprites are getting resurrected).

basewindow and setVisible
LPP-5127: "setVisible(false)" is already called from "LzView#destroy(..)", is this sufficient enough?
- and I don't see any leaks in "canvas.options.windowlist", it's an empty Array.

LPP-4879: to fix this bug, I've changed "LzView#destroy(..)" to call "this.sprite.destroy(true)", beforehand it was "this.sprite.destroy(recursiveCall)", but "recursiveCall" was always "false" (if "recursiveCall" is "true", the function returns at an earlier stage). I've also put the unregistering of "updatePlayDel" and "doQueuedDel" into "LzSprite#predestroy()", because every LzView calls at first "LzSprite#predestroy()", then it removes itself from its immediateparent's subviews-Array and then it invokes "super.destroy.apply(this, arguments)" etc. After all this destroy-recursion has ended, the top-view (the one with "recursiveCall == false") will call "LzSprite#destroy(..)" and then we'd like to have again a destroy-recursion, but now only over the LzSprites. But this won't happen, because "LzSprite#destroy(..)" uses "this.owner.subviews" to iterate over all sub-sprites. But "this.owner.subviews" is actually an empty Array at this stage (see above). [Only SWF, DHTML uses an extra "__children" Array]
[This only true for non replication, see "LzReplicationManager#destroyClone(..)"]
Maybe we need to have this unregistering two times, once for "predestroy" and again for "destroy", just to be save..

Why do we need to delete "basecomponent#_mousedownDel", "basewindow#mousedown_del" and "basewindow#mouseup_del"? These are not cross-instance delegates, so I don't see any leak-possibility!? But, we could change them to "<handler name='onfoo' method='methodFoo' />", because that's less scripting and more xml-style and this way they'd get cleaned automatically by "LzNode#destroy(..)".

I think there is confusion about what recursive means to destroy. Originally, it meant:

destroy is being called on a child node from a parent node that is being destroyed

See LzNode#destroy ca. line 1729 where it destroys the subnodes.

The purpose of that was to prevent the subnodes from trying to remove themselves from the parent that was in the act of destroying them.

I think if we structure this correctly, there should be no need for LzSprite#destroy to do any recursing, since each node should just destroy its own sprite. This may simply mean that the call to LzSprite#destroy is on the wrong side of the early return when recursive is true.

In running Phil's test case, I have found that the main cause of leaks is that the SWF sprite is using the LZX event system, but does not use the 'secret' __LZdeleted flag to prevent events from firing on deleted objects. As a result, in Phil's test case the deleted sprites are getting 'resurrected' by events firing on them after they have been deleted.

I have a patch in progress that I will send for review, as soon as I can clean up a few rough edges. (I got sidetracked trying to make the memory tracing tools work in DHTML...)

As to the mouse*_del's, it's generally just good practice to delete any delegates when you are deleted.

> I think if we structure this correctly, there should be no need for LzSprite#destroy to do any recursing, since each node should just destroy its own sprite.

Not sure about that, see LPP-5173. I've removed the recursion from "LzSprite#destroy" (as) with the change for LPP-4879, but it broke Flash.

> I think there is confusion about what recursive means to destroy. Originally, it meant:
> destroy is being called on a child node from a parent node that is being destroyed

Yes, I know.
In the LZX-world (LzNode, LzView) it's just a flag to say: "hey node, don't do any fancy stuff now, your parent node is getting destroyed, too!"
Whereas in the sprite-world it is used for... merely nothing. Apparently, it was just taken from the LZX-world to have...?


> The purpose of that was to prevent the subnodes from trying to remove themselves from the parent that was in the act of destroying them.

In LzNode#destroy, the recursiveCall-flag is unused (we just pass recursiveCall:=true to the subnodes)
In LzView#destroy, it's just used to trigger the sprite-destruction, the setVisible-call and the __LZoutliewidth/__LZoutlieheight/onremovesubview stuff.

So, do you mean, that we also need to use recursiveCall-flag at the very beginning of LzView#destroy:
if ( !recursiveCall && this.addedToParent ){
    var svs = this.immediateparent.subviews;
    ...
}

and in LzNode#destroy (near the end):
if (!recursiveCall && this.immediateparent && this.immediateparent.subnodes) {
    for( var i = this.immediateparent.subnodes.length - 1; i >= 0; i-- ){
        ...
   }
}


Maybe, we need take into our consideration that a bunch of components overrode "destroy", but they didn't pass on the recursiveCall-flag. Therefore, someone could have changed the original recursiveCall-logic, because destroy wasn't working like it should...!

Within the changeset for LPP-4879, I've updated a couple of them (many have been updated earlier).
So, instead of:
<method name="destroy" args="recursiveCall" >
  //super.destroy(recursiveCall);//no Function#apply in OL3.x
  super.destroy.apply(this, arguments);
</method>

there was only:
<method name="destroy" >
  super.destroy();
</method>

This issue is also reported at LPP-5149.

I'm attaching a copy of LzSprite.js that contains a fix for the memory leak in dhtml. I'm posting this so we don't duplicate effort. I'm not done testing it, but it appears to work ok.

I'm taking this because Max is not available at this time.

I think you are inventing the same solution I am. I added this to LzSprite#destroy:

    // Remove from parent if the parent is not going to be GC-ed
    if (! this.__parent.__LZdeleted) {
      var pc = this.__parent.__children;
      for (var i = pc.length - 1; i >= 0; i--) {
        if (pc[i] === this) {
          pc.splice(i, 1);
          break;
        }
      }
    }

r7518 | ptw | 2007-12-12 08:13:58 -0500 (Wed, 12 Dec 2007) | 55 lines
Changed paths:
   M /openlaszlo/trunk/WEB-INF/lps/lfc/core/LzNode.lzs
   M /openlaszlo/trunk/WEB-INF/lps/lfc/data/LzReplicationManager.lzs
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/svg/LzSprite.js
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/swf/LzMakeLoadSprite.as
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/swf/LzSprite.as
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/swf9/LzMakeLoadSprite.lzs
   M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/swf9/LzSprite.lzs
   M /openlaszlo/trunk/WEB-INF/lps/lfc/services/LzIdle.lzs
   M /openlaszlo/trunk/WEB-INF/lps/lfc/views/LaszloView.lzs
   M /openlaszlo/trunk/lps/components/debugger/newcontent.lzx
   M /openlaszlo/trunk/lps/components/debugger/scrollingtext.lzx

Change 20071209-ptw-k by ptw@dueling-banjos.local on 2007-12-09 09:32:18 EST
    in /Users/ptw/OpenLaszlo/ringding-2
    for http://svn.openlaszlo.org/openlaszlo/trunk

Summary: Fix Sprite memory Leaks

Bugs Fixed:
LPP-5217 'windows can leak memory

Technical Reviewer: a.bargull@intensis.de (message://<475F0E97.90905@intensis.de>)
QA Reviewer: philip@pbrdev.com (message://<20071210171315.089691DF284@hemicuda.laszlosystems.com>)
Doc Reviewer: (pending)

Details:

    LzMakeLoadSprite.*, LzSprite.*: Eliminate recursion argument. Not
    used for sprites.

    LzSprite.*: In #destroy set __LZdeleted flag to prevent events
    from resurrecting you. No need for recursive sprite destruction
    now. Move delegate removal back to destroy from predestroy.

    LzSprite.js: If parent is not being deleted, remove yourself from
    your parent's childnodes array.

    LzIdle: Be careful to create coi Array on instance, not prototype.

    LzNode: Comment and simplify $once and $always processing. Add
    comment to explain importance of __LzDeleted flag.

    LaszloView: Destroy the sprite _before_ you check for a recursive
    call.

    LzReplicationManager: Remove superfluous call to LzSprite#destroy.

    newcontent, scrollingtext: Remove unused id (that causes a leak).

Tests:
    I modified Phil's test case to only create/destroy 1 window per
    click. The test methodology is:

    1. Start test
    2. Click twice to create any shared substrate
    3. Debug.markObjects()
    4. Click once
    5. Debug.findNewObjects()
    6. Debug.whyAlive()

    Result: Before change, many leaked window sprites. After change, no
    leaked window sprites.

    NOTE: There are still some other minor leaks that I have not
    attempted to address here, but will report as a separate bug.

Migrated to 4.0.5.2:

r7557 | ptw | 2007-12-16 17:27:13 -0500 (Sun, 16 Dec 2007) | 21 lines
Changed paths:
   M /openlaszlo/branches/4.0.5.2
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/core/LzNode.lzs
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/data/LzReplicationManager.lzs
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/kernel/svg/LzSprite.js
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/kernel/swf/LzMakeLoadSprite.as
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/kernel/swf/LzSprite.as
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/services/LzIdle.lzs
   M /openlaszlo/branches/4.0.5.2/WEB-INF/lps/lfc/views/LaszloView.lzs
   M /openlaszlo/branches/4.0.5.2/lps/components/debugger/newcontent.lzx
   M /openlaszlo/branches/4.0.5.2/lps/components/debugger/scrollingtext.lzx

Change 20071216-ptw-3 by ptw@dueling-banjos.local on 2007-12-16 10:49:44 EST
    in /Users/ptw/OpenLaszlo/4.0.5.2
    for http://svn.openlaszlo.org/openlaszlo/branches/4.0.5.2

Summary: Fix Sprite memory Leaks

Bugs Fixed:
LPP-5217 'windows can leak memory

Technical Reviewer: philip@pbrdev.com (pending)
QA Reviewer: mamye@laszlosystems.com (pending)

Details:
    Merged revisions 7518 via svnmerge from
    http://svn.openlaszlo.org/openlaszlo/trunk

Tests:
    Test case from bug report. Before: many leaked LzSprite objects,
    after: none.

Test case from LPP-5213:

<canvas>

  <window visible="false" />

  <view>
    <simplelayout axis="x" spacing="10"/>

    <button text="Open/Close windows (leaks)" >
      <attribute name="counter" type="number" value="0"/>

      <method name="test">
        var win = new lz.window(canvas,{title:'new', resizable:true, closeable:true});
        win.open();
        win.close();
        win.destroy();
        delete win;
      </method>

      <handler name="onclick">
        <![CDATA[
          for (var i = 0; i < 100; i++) {
            this.test();
            this.counter++;
            Debug.write(this.counter);
          }
        ]]>
      </handler>
    </button>

  </view>
</canvas>

I was wrong when I said:

" NOTE: There are still some other minor leaks that I have not
    attempted to address here, but will report as a separate bug. "

The leak detector had a long-standing bug, LPP-2071, which I just fixed. With that bug fixed, running the above test yields:

lzx> Debug.whyAlive()
89 smoots [5 objects @ ~18 smoots each]:
global.spriteroot.$m2.debugloader.loadmc1.reqobj: (£52) «Object#9| {_dbg_check: 52, _dbg_smoots: ...»
global.LzFocus.cseldest.sprite.__LZtextclip.__handlelostFocusdel: (£21) «LzDelegate#12| [].__handlelostFocus()»
global.LzKeys.downKeysArray: (£8) «Array(2)#15| [16, 68]»
global.LzIdle.coi: (£8) «Array(0)#18| []»
.: (£0) «¿movieclip?»
«__LzLeaks(5)#7| 89 smoots [5 objects @ ~18 smoots each]»
lzx>

These objects are all objects that are created by me typing into the debugger to trace the memory. So the only leak left is to make the leak detector not report the objects that are created by typing in the debugger.

Also fixed in Wafflecone HEAD (destined to become 4.0.8):

r7705 | ptw | 2008-01-02 17:23:24 -0500 (Wed, 02 Jan 2008) | 99 lines
Changed paths:
   M /openlaszlo/branches/wafflecone
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/core/LzNode.lzs
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/data/LzReplicationManager.lzs
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/kernel/svg/LzSprite.js
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/kernel/swf/LzMakeLoadSprite.as
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/kernel/swf/LzSprite.as
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/services/LzIdle.lzs
   M /openlaszlo/branches/wafflecone/WEB-INF/lps/lfc/views/LaszloView.lzs
   M /openlaszlo/branches/wafflecone/lps/components/debugger/newcontent.lzx
   M /openlaszlo/branches/wafflecone/lps/components/debugger/scrollingtext.lzx

Change 20080102-ptw-l by ptw@dueling-banjos.local on 2008-01-02 16:13:33 EST
    in /Users/ptw/OpenLaszlo/wafflecone
    for http://svn.openlaszlo.org/openlaszlo/branches/wafflecone

Summary: Merged revisions 7518,7575 via svnmerge from
http://svn.openlaszlo.org/openlaszlo/trunk

QA Reviewer: mamye (pending)

Details:
........
  r7518 | ptw | 2007-12-12 08:13:58 -0500 (Wed, 12 Dec 2007) | 55 lines
  
  Change 20071209-ptw-k by ptw@dueling-banjos.local on 2007-12-09 09:32:18 EST
      in /Users/ptw/OpenLaszlo/ringding-2
      for http://svn.openlaszlo.org/openlaszlo/trunk
  
  Summary: Fix Sprite memory Leaks
  
  Bugs Fixed:
  LPP-5217 'windows can leak memory
  
  Technical Reviewer: a.bargull@intensis.de (message://<475F0E97.90905@intensis.de>)
  QA Reviewer: philip@pbrdev.com (message://<20071210171315.089691DF284@hemicuda.laszlosystems.com>)
  Doc Reviewer: (pending)
  
  Details:
  
      LzMakeLoadSprite.*, LzSprite.*: Eliminate recursion argument. Not
      used for sprites.
  
      LzSprite.*: In #destroy set __LZdeleted flag to prevent events
      from resurrecting you. No need for recursive sprite destruction
      now. Move delegate removal back to destroy from predestroy.
  
      LzSprite.js: If parent is not being deleted, remove yourself from
      your parent's childnodes array.
  
      LzIdle: Be careful to create coi Array on instance, not prototype.
  
      LzNode: Comment and simplify $once and $always processing. Add
      comment to explain importance of __LzDeleted flag.
  
      LaszloView: Destroy the sprite _before_ you check for a recursive
      call.
  
      LzReplicationManager: Remove superfluous call to LzSprite#destroy.
  
      newcontent, scrollingtext: Remove unused id (that causes a leak).
  
  Tests:
      I modified Phil's test case to only create/destroy 1 window per
      click. The test methodology is:
  
      1. Start test
      2. Click twice to create any shared substrate
      3. Debug.markObjects()
      4. Click once
      5. Debug.findNewObjects()
      6. Debug.whyAlive()
  
      Result: Before change, many leaked window sprites. After change, no
      leaked window sprites.
  
      NOTE: There are still some other minor leaks that I have not
      attempted to address here, but will report as a separate bug.
........
  r7575 | ptw | 2007-12-17 20:14:14 -0500 (Mon, 17 Dec 2007) | 25 lines
  
  Change 20071217-ptw-P by ptw@dueling-banjos.local on 2007-12-17 17:56:31 EST
      in /Users/ptw/OpenLaszlo/ringding-2
      for http://svn.openlaszlo.org/openlaszlo/trunk
  
  Summary: Brain-oh in LzSprite.js
  
  Bugs Fixed:
  LPP-5274 'Error: this.__parent has no properties (DHTML)'
  
  Technical Reviewer: philip@pbrdev.com (pending)
  QA Reviewer: maymye (pending)
  
  Details:
      Test for __parent before using it
  
  Tests:
      Environment: OS X, FF2
      Launch laszlo-explorer in flash
      Choose Demos in navbar
      Choose Amazon
      Select the Launch in DHTML button
      Observe green checkmark at bottom right of Amazon window, NO LONGER
      turns to a red X
........
    

Tests:
    Test case from LPP-5217, Explorer/Demos/Amazon/DHTML

Fixed in Eskimo Pie, not Cranberry

(wafflecone local build r7719 - 4.0.8 RC)
Testcase:
<canvas>

  <window visible="false" />

  <view>
    <simplelayout axis="x" spacing="10"/>

    <button text="Open/Close windows (leaks)" >
      <attribute name="counter" type="number" value="0"/>

      <method name="test">
        var win = new lz.window(canvas,{title:'new', resizable:true, closeable:true});
        win.open();
        win.close();
        win.destroy();
        delete win;
      </method>

      <handler name="onclick">
        <![CDATA[
          for (var i = 0; i < 100; i++) {
            this.test();
            this.counter++;
            Debug.write(this.counter);
          }
        ]]>
      </handler>
    </button>

  </view>
</canvas>

Results:
lzx> Debug.printLength = Infinity
Infinity
lzx> Debug.markObjects()
Marking objects ...
lzx> DEBUG: 16 loops @ 670 iterations, 1256.25 milliseconds
 ... done!
1
2
3
...
96
97
98
99
100
lzx> Debug.findNewObjects()
Finding new objects ...
lzx> DEBUG: 17 loops @ 642 iterations, 1297.76 milliseconds
 ... done!
lzx> Debug.whyAlive()
4037 smoots [18 objects @ ~224 smoots each]:
global.window.prototype.classChildren.11.children.0.attrs.$m23._dbg_owner: (£2154) «lz.view#4»
global.canvas.subviews.0.title_area.controls.closeable.subh.0.attrs.$m22._dbg_owner: (£1535) «lz.basebutton#7»
global.LzFocus.lastfocus.__LZviewLinks: (£60) «Object#9| {_dbg_check: 60, _dbg_smoots: ...»
global.spriteroot.$m2.debugloader.loadmc1.reqobj: (£50) «Object#12| {_dbg_check: 50, _dbg_smoots:...»
global.canvas.subviews.1.ony: (£27) «LzEvent#15| «lz.view».ony»
global.LzFocus.lastfocus.ony: (£27) «LzEvent#18| «lz.button».ony»
global.LzFocus.lastfocus.onx: (£27) «LzEvent#20| «lz.button».onx»
global.canvas.subviews.1.onx: (£27) «LzEvent#22| «lz.view».onx»
global.window.prototype.classChildren.11.children.0.attrs.$m26._dbg_typename: (£18) «Function#25| _dbg_typename»
global.canvas.subviews.0.title_area.controls.closeable.subh.0.attrs.$m22._dbg_typename: (£18) «Function#27| _dbg_typename»
... [8 more @ ~12 smoots each]
«__LzLeaks(18)#2| 4037 smoots [18 objects @ ~224 smoots each]»
lzx> Debug.format("%s", _.toString())
[object Object]
lzx>

Need to test in trunk for ringding.

(trunk 4 build r7775)
Fixed in ringding too.

lzx> Debug.printLength = Infinity
Infinity
lzx> Debug.markObjects()
Marking objects ...
lzx> DEBUG: 15 loops @ 717 iterations, 1290.53 milliseconds
 ... done!
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
lzx> Debug.findNewObjects()
Finding new objects ...
lzx> DEBUG: 18 loops @ 609 iterations, 1267.44 milliseconds
 ... done!
lzx> Debug.whyAlive()
4086 smoots [20 objects @ ~204 smoots each]:
global.window.prototype.classChildren.11.children.0.attrs.$m25._dbg_owner: (£2154) «lz.view#4»
global.canvas.subviews.0.title_area.controls.closeable.subh.0.attrs.$m22._dbg_owner: (£1529) «lz.basebutton#7»
global.canvas.subviews.1.__LZoutlieheight.__LZviewLinks: (£60) «Object#10| {_dbg_check: 60, _dbg_smoots:...»
global.spriteroot.$m2.debugloader.loadmc1.reqobj: (£49) «Object#13| {_dbg_check: 49, _dbg_smoots:...»
global.canvas.__focus._xydelegate.7: (£29) «LzEvent#17| #Debug.ony»
global.canvas.__focus._xydelegate.6: (£29) «LzEvent#19| #Debug.onx»
global.canvas.subviews.1.ony: (£27) «LzEvent#22| «lz.view».ony»
global.canvas.subviews.1.__LZoutlieheight.onx: (£27) «LzEvent#24| «lz.button».onx»
global.canvas.subviews.1.__LZoutlieheight.ony: (£27) «LzEvent#26| «lz.button».ony»
global.canvas.subviews.1.onx: (£27) «LzEvent#28| «lz.view».onx»
... [10 more @ ~13 smoots each]
«__LzLeaks(20)#2| 4086 smoots [20 objects @ ~204 smoots each]»
lzx> Debug.format("%s", _.toString())
[object Object]
lzx> Debug.versionInfo()
URL: http://127.0.0.1:8080/trunk/my-apps/5217.lzx?lzt=swf&lzr=swf8&debug=true&lzbacktrace=false
Version: 4.1.x.0
Release: Latest
Build: 7775 C:\cygwin\home\mamye\src\svn\openlaszlo\trunk
Date: 2008-01-09T15:53:12-0500
Target: swf8
Runtime: 8.24
lzx>