New Leak Found: Keeping references to destroyed objects.

Discussion in 'JASS Zone' started by Jesus4Lyf, Sep 9, 2009.

  1. Jesus4Lyf

    Jesus4Lyf Good Idea™

    Ratings:
    +394 / 0 / -0
    These are memory leaks (but not handle id leaks). You must check the process list to see it leak.
    Credits to Troll-Brain for finding the original leak.

    This leaks:
    JASS:
    library Test initializer init
    
    globals
        private group G1
        private group G2
        private group GROUP=CreateGroup()
        private trigger Trig
    endglobals
    
    private function Actions takes nothing returns nothing
        call BJDebugMsg("Start")
        call EnableTrigger(Trig)
        call TriggerSleepAction(10.)
        call DisableTrigger(Trig)
        call BJDebugMsg("End")
    endfunction
    
    private function DoTheTest takes nothing returns nothing
        //call BJDebugMsg("run")
        set G1=null // With or without this line.
        set G1=CreateGroup()
        call DestroyGroup(G1)
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        set Trig = CreateTrigger()
        
        call TriggerRegisterTimerEvent(Trig,0.0,true)
        call TriggerAddAction(Trig,function DoTheTest)
        call DisableTrigger(Trig)
        
        call TriggerRegisterPlayerEventEndCinematic(trig,Player(0))
        call TriggerAddAction(trig, function Actions)
    endfunction
    
    endlibrary
    In fact, even this leaks:
    JASS:
    library Test initializer init
    
    globals
        private group G1
        private group G2
        private group GROUP=CreateGroup()
        private trigger Trig
    endglobals
    
    private function Actions takes nothing returns nothing
        call BJDebugMsg("Start")
        call EnableTrigger(Trig)
        call TriggerSleepAction(10.)
        call DisableTrigger(Trig)
        call BJDebugMsg("End")
    endfunction
    
    globals
        boolean k=false
    endglobals
    
    private function DoTheTest takes nothing returns nothing
        //call BJDebugMsg("run")
        if k then
            set G1=null
        else
            set G1=CreateGroup()
            call DestroyGroup(G1)
        endif
        set k = not k
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        set Trig = CreateTrigger()
        
        call TriggerRegisterTimerEvent(Trig,0.01,true)
        call TriggerAddAction(Trig,function DoTheTest)
        call DisableTrigger(Trig)
        
        call TriggerRegisterPlayerEventEndCinematic(trig,Player(0))
        call TriggerAddAction(trig, function Actions)
    endfunction
    
    endlibrary
    This does not leak:
    JASS:
    library Test initializer init
    
    globals
        private group G1
        private group G2
        private group GROUP=CreateGroup()
        private trigger Trig
    endglobals
    
    private function Actions takes nothing returns nothing
        call BJDebugMsg("Start")
        call EnableTrigger(Trig)
        call TriggerSleepAction(10.)
        call DisableTrigger(Trig)
        call BJDebugMsg("End")
    endfunction
    
    private function DoTheTest takes nothing returns nothing
        //call BJDebugMsg("run")
        set G1=CreateGroup()
        call DestroyGroup(G1)
        set G1=null
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        set Trig = CreateTrigger()
        
        call TriggerRegisterTimerEvent(Trig,0.0,true)
        call TriggerAddAction(Trig,function DoTheTest)
        call DisableTrigger(Trig)
        
        call TriggerRegisterPlayerEventEndCinematic(trig,Player(0))
        call TriggerAddAction(trig, function Actions)
    endfunction
    
    endlibrary
    What's the exact cause? Discuss. I will update this post as conclusions are reached and verified.

    This is a minor leak.

    Conclusion:
    Not allowing a handle id to be recycled immediately after destruction causes a memory leak.
     
  2. Jesus4Lyf

    Jesus4Lyf Good Idea™

    Ratings:
    +394 / 0 / -0
    I have a new conclusion. We know that the handle id recycler will pick up a handle at any point after it has been destroyed, when its reference count hits 0.

    Well, it seems if that is not immediately after it is destroyed, then it causes a leak.

    I conclude this because setting the variable to "GROUP" (a constant group) also stops the leak.

    Edit: Yep.
    Does not leak:
    JASS:
    library Test initializer init
    
    globals
        private group G1
        private group G2
        private group GROUP=CreateGroup()
        private trigger Trig
    endglobals
    
    private function Actions takes nothing returns nothing
        call BJDebugMsg("Start")
        call EnableTrigger(Trig)
        call TriggerSleepAction(10.)
        call DisableTrigger(Trig)
        call BJDebugMsg("End")
    endfunction
    
    private function OtherTest takes nothing returns nothing
        local group g=CreateGroup()
        call DestroyGroup(g)
        set g=null
        call TriggerSleepAction(0)
    endfunction
    
    private function DoTheTest takes nothing returns nothing
        //call BJDebugMsg("run")
        call OtherTest.execute()
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        set Trig = CreateTrigger()
        
        call TriggerRegisterTimerEvent(Trig,0.0,true)
        call TriggerAddAction(Trig,function DoTheTest)
        call DisableTrigger(Trig)
        
        call TriggerRegisterPlayerEventEndCinematic(trig,Player(0))
        call TriggerAddAction(trig, function Actions)
    endfunction
    
    endlibrary
    Does leak:
    JASS:
    library Test initializer init
    
    globals
        private group G1
        private group G2
        private group GROUP=CreateGroup()
        private trigger Trig
    endglobals
    
    private function Actions takes nothing returns nothing
        call BJDebugMsg("Start")
        call EnableTrigger(Trig)
        call TriggerSleepAction(10.)
        call DisableTrigger(Trig)
        call BJDebugMsg("End")
    endfunction
    
    private function OtherTest takes nothing returns nothing
        local group g=CreateGroup()
        call DestroyGroup(g)
        call TriggerSleepAction(0)
        set g=null
    endfunction
    
    private function DoTheTest takes nothing returns nothing
        //call BJDebugMsg("run")
        call OtherTest.execute()
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        set Trig = CreateTrigger()
        
        call TriggerRegisterTimerEvent(Trig,0.0,true)
        call TriggerAddAction(Trig,function DoTheTest)
        call DisableTrigger(Trig)
        
        call TriggerRegisterPlayerEventEndCinematic(trig,Player(0))
        call TriggerAddAction(trig, function Actions)
    endfunction
    
    endlibrary
     
  3. T.s.e

    T.s.e Wish I was old and a little sentimental

    Ratings:
    +133 / 0 / -0
    I still don't see why you would want to destroy a global group, but it is probably an example that applies to handles in general?
     
  4. Jesus4Lyf

    Jesus4Lyf Good Idea™

    Ratings:
    +394 / 0 / -0
    Occurs for all agents. Local and global.

    Updated the original post with the new conclusion.
     
  5. Zwiebelchen

    Zwiebelchen You can change this now in User CP.

    Ratings:
    +60 / 0 / -0
    So the conclusion is: Even globals have to be nulled after destruction.

    One question: Does this leak? If yes, then globals have to be nulled immediately and not only inside the same instance.

    JASS:
    
    set G1 = CreateGroup()
    call DestroyGroup(G1)
    call DoNothing()
    set G1 = null
     
  6. Jesus4Lyf

    Jesus4Lyf Good Idea™

    Ratings:
    +394 / 0 / -0
    This also leaks (demo using hashtables):
    JASS:
    library Test initializer init
    
    globals
        private group G1
        private group G2
        private group GROUP=CreateGroup()
        private trigger Trig
        private hashtable H=InitHashtable()
    endglobals
    
    private function Actions takes nothing returns nothing
        call BJDebugMsg("Start")
        call EnableTrigger(Trig)
        call TriggerSleepAction(10.)
        call DisableTrigger(Trig)
        call BJDebugMsg("End")
    endfunction
    
    private function DoTheTest takes nothing returns nothing
        call SaveGroupHandle(H,0,0,CreateGroup())
        call DestroyGroup(LoadGroupHandle(H,0,0))
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        set Trig = CreateTrigger()
        
        call TriggerRegisterTimerEvent(Trig,0.0,true)
        call TriggerAddAction(Trig,function DoTheTest)
        call DisableTrigger(Trig)
        
        call TriggerRegisterPlayerEventEndCinematic(trig,Player(0))
        call TriggerAddAction(trig, function Actions)
    endfunction
    
    endlibrary
    This does not leak (hashtables):
    JASS:
    private function DoTheTest takes nothing returns nothing
        call SaveGroupHandle(H,0,0,CreateGroup())
        call DestroyGroup(LoadGroupHandle(H,0,0))
        call SaveGroupHandle(H,0,0,GROUP)
    endfunction

    Neither does this:
    JASS:
    private function DoTheTest takes nothing returns nothing
        call SaveGroupHandle(H,0,0,CreateGroup())
        call DestroyGroup(LoadGroupHandle(H,0,0))
        call RemoveSavedHandle(H,0,0)
    endfunction

    >One question: Does this leak? If yes, then globals have to be nulled immediately
    No, it does not (tested). But remember, nulling after a wait does (if the handle is removed during the wait).

    So the conclusion is solid?
     
  7. kingkingyyk3

    kingkingyyk3 Visitor (Welcome to the Jungle, Baby!)

    Ratings:
    +216 / 0 / -0
    Tested. Setting a variable(contains destroyed value) to a new value will cause leaking.
     
  8. Jesus4Lyf

    Jesus4Lyf Good Idea™

    Ratings:
    +394 / 0 / -0
    ...
    ...
    Unless it is done after destruction and before a wait.
     
  9. Azlier

    Azlier Old World Ghost

    Ratings:
    +461 / 0 / -0
    Blizzfail detected.
     
  10. kingkingyyk3

    kingkingyyk3 Visitor (Welcome to the Jungle, Baby!)

    Ratings:
    +216 / 0 / -0
    1.25 patch? o_O
     
  11. Azlier

    Azlier Old World Ghost

    Ratings:
    +461 / 0 / -0
    Nay, Blizzard only fixes things that affect their income. Virus maps, yeah, suing could happen there. But a minor memory leak that people found out about eight years later? I don't think so.
     
  12. T.s.e

    T.s.e Wish I was old and a little sentimental

    Ratings:
    +133 / 0 / -0
    Then why did they fix local masking? I can't see how that would affect their income.
     
  13. Viikuna

    Viikuna No Marlo no game.

    Ratings:
    +265 / 0 / -0
    Actually, according to Vexorian, local masking works perfectly now. ( link )

    Noone really knows what Blizzard is doing and why...
     
  14. Azlier

    Azlier Old World Ghost

    Ratings:
    +461 / 0 / -0
    >Then why did they fix local masking?

    Because apparently typecasting could be done via shadowing. So Blizzard decided to fixit. Or something.
     
  15. uberfoop

    uberfoop ~=Admiral Stukov=~

    Ratings:
    +176 / 0 / -0
    I guess that at least (partially) explains the slow memory increases of supposedly leak free maps that aren't increasing in active object count.

    On the plus side, these leaks are tiny and generally preventable.
     
  16. Troll-Brain

    Troll-Brain You can change this now in User CP.

    Ratings:
    +85 / 0 / -0
    Yes it could be done :

    http://www.wc3c.net/showthread.php?t=98230
     
  17. Lyerae

    Lyerae I keep popping up on this site from time to time.

    Ratings:
    +105 / 0 / -0
    Oh hey look, a new leak...
    Just another thing I need to learn how to fix. OH WELL.
     
  18. Troll-Brain

    Troll-Brain You can change this now in User CP.

    Ratings:
    +85 / 0 / -0
    It's not that hard, just set to null the handle variable just after you have destroyed the handle, or flush gamecache/hashtable if you used it instead of a variable.

    According to Jesus4Lyf you might can do it after few lines of code without waits, but i recommend to set to null in the line just below the handle destruction, just to be sure, at least it's more likely you won't forget it.
     
  19. Lyerae

    Lyerae I keep popping up on this site from time to time.

    Ratings:
    +105 / 0 / -0
    Yeah, well... It's still another thing to remember. And I'm really bad at remembering...
     
  20. uberfoop

    uberfoop ~=Admiral Stukov=~

    Ratings:
    +176 / 0 / -0
    If you just recycle what can be recycled in a practical manner you should be fine. That's more efficient anyway than create and destroy, and results in cleaner end code (ie no need to null anything). Global group + GroupClear for instant-use groups, GroupUtils or some other recycler for other groups, TU and maybe T32 for timers depending on what you are doing/need, etc.
     

Share This Page