xkos / wagic

Automatically exported from code.google.com/p/wagic
0 stars 0 forks source link

mem leak with target chooser #585

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Play match with the following deck.  Finish match and quit game.  Mem leak 
message displayed in console.  

Tracing it back I found this bug was introduced somewhere between 2936 and 
2941.  

#NAME: Pauper Blender
#MANA:111100
Exclude  (INV) *3
Wild Mongrel     (ODY) *4
Deep Analysis    (TOR) *3
Tin Street Hooligan  (GPT) *2
Simic Growth Chamber     (DIS) *2
Izzet Boilerworks    (GPT) *3
Looter il-Kor    (TSP) *4
Mulldrifter  (LRW) *4
Flame Jab    (EVE) *1
Burst Lightning  (ZEN) *3
Evolving Wilds   (ROE) *2
Lightning Bolt   (M11) *4
Terramorphic Expanse     (M11) *4
Gruul Turf   (HOP) *2
Negate   (M11) *3
Forest   (SOM) *3
Island   (SOM) *4
Mountain     (SOM) *4

What is the expected output? What do you see instead?

Please use labels and text to provide additional information.

Original issue reported on code.google.com by techdragon.nguyen@gmail.com on 5 Feb 2011 at 7:41

Attachments:

GoogleCodeExporter commented 8 years ago
quick question, which mthod did you use to narrow it down to a change between 
those revs? btw...for the sake of sanity, 2936-->current....

Original comment by omegabla...@gmail.com on 5 Feb 2011 at 8:49

GoogleCodeExporter commented 8 years ago
Can't repro.  Are you sure you exited the game normally instead of clicking the 
top right 'X', which preempts all the shutdown calls..?

Original comment by wrenc...@gmail.com on 10 Apr 2011 at 2:44

GoogleCodeExporter commented 8 years ago
I'm positive. This happened to very regularly with that specific deck during 
that time.  Haven't tried since then.  I did not used the close window button 
to exit the app. 

Original comment by techdragon.nguyen@gmail.com on 10 Apr 2011 at 4:06

GoogleCodeExporter commented 8 years ago
i also have not been able to repro this, unless you can narrow down the the 
cause on your end, im going to go ahead and close this for now.

Original comment by omegabla...@gmail.com on 11 Apr 2011 at 2:17

GoogleCodeExporter commented 8 years ago
It's pretty easy to reproduce for me anyways.  I believe it has something to do 
with the bounce lands.  It seems that there is some combination that creates a 
mem leak in TargetChooser.cpp (line 622) now.  Out of 10 games I get the mem 
leak 5 times.  Each time there was a bounce land from that deck in play.  
However, that isn't sufficient since the games that didn't have the leak also 
had a bounce land in play.  It could be combination of the bounce lands and a 
particular card from the deck, like Wild Mongrel or mulldrifter.  These cards 
either sacrifice or give card advantage.  Not proven just a theory.  However, I 
know this bug exists.  with that deck I posted.  

I'm reopening this issue as I can still create it on a new checkout, no images, 
on the very first game played.

Original comment by techdragon.nguyen@gmail.com on 11 Apr 2011 at 8:08

GoogleCodeExporter commented 8 years ago
it is really hard to shallow this pill without any water...
i personally have not been able to repro this bug, and theory crafting isn't 
going to get us anywhere near close to a repro on this issue, have anyone else 
been able to repro this?

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 6:24

GoogleCodeExporter commented 8 years ago
Out of 10 games I get the mem leak 5 times...since you can repro this at a 
pretty decent rate, how about you start removing cards a playset at a time, 
until you remove the one causing this. so far, you are the only one able to 
repro this and having a ticket open that provides an entire deck as a bug 
report, is invalid...

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 6:29

GoogleCodeExporter commented 8 years ago
Tin Street Hooligan

this is the card causing it.
not the lands.
100% repro, take your deck and in the primitive of "forest"
add this
abilities=cantmilllose
auto={0}:all(*|mydeck) moveto(mybattlefield)
then close out, now where it saids the memleak, double click it, it takes you 
to tc= typeName...break point, restart start another match, repeat, remove 
cards until you only have the forest and the card which caused the memleak...

btw, MTGAbility memleaks, and TC memleaks are never caused by "a series of 
events...it is always 1 card, and 1 line....hope that helps you narrow down 
reports like this one.

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 7:29

GoogleCodeExporter commented 8 years ago
also this was not introduced in the revs you are claiming...it was introduced 
when this
const string Constants::kAlternativeKeyword = "alternative";
was introduced. and the way that these abilities get added was changed from the 
original version to what it is in current.

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 7:32

GoogleCodeExporter commented 8 years ago
    if ((costType > -1) && (!keyword.empty()))
    {
        if (spell && spell->FullfilledAlternateCost(costType))
        {
            string s1 = s.substr(keyword.length());
            return parseMagicLine(s1, id, spell, card);
        }
        DebugTrace("INFO parseMagicLine: Alternative Cost was not fulfilled for " << s);
        return NULL;
    }

heres your memleak.

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 7:33

GoogleCodeExporter commented 8 years ago
you return null, becuase it was not paid, however WAY before anything else was 
parsed, "target(" was found in the string...and a TC was created for it...you 
never delete the tc so you end up with a memleak....

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 7:37

GoogleCodeExporter commented 8 years ago
fixed.

Original comment by omegabla...@gmail.com on 14 Apr 2011 at 7:40

GoogleCodeExporter commented 8 years ago
I only pointed out that those revs were where I was able to consistently
reproduce the error.  The revs prior to this, I was unable to generate the
error.  I had only focused on the current release at the time.  I assumed
that something like this would have been caught a long time ago, considering
that bug addition was added in 12.1 i believe, two or three revs ago.  Just
argues that we need to improve the test mechanisms in the game.  So bugs
like this won't happen.

Nice find.

Original comment by techdragon.nguyen@gmail.com on 15 Apr 2011 at 2:35