bugGnash - The GNU Flash player - Bugs: bug #32521, SIGKILL after ever-growing...

 
 

bug #32521: SIGKILL after ever-growing resource usage

Submitted by:  Bastiaan Jacques <bjacques>
Submitted on:  Thu 17 Feb 2011 05:54:26 PM UTC  
 
Category: ActionScriptSeverity: 5 - Blocker
Release: masterStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 12 Sep 2011 12:16:35 PM UTC, comment #27:

this is clearly fixed.

Sandro Santilli <strk>
Project Member
Wed 30 Mar 2011 07:01:31 AM UTC, comment #26:

With 9c79a4142f841cb587053fa68a77aea12a3d8eb0 this case becomes less of a problem, in that the first time the limit is hit scripts are completely disabled, so memory stops growing.
Also, the same commit reduces the timeout from 40 seconds to 15 seconds, as reported to be needed.

Take a look.

On another point: did you check with jemalloc disabled ? as I never see the memory coming down, even after GC runs. I think this same behavior was seens in another ticket and wondering if it's memory arenas never getting released or what.

Sandro Santilli <strk>
Project Member
Tue 15 Mar 2011 03:42:49 AM UTC, comment #25:

I'm not inclined to agree with your reasoning, because I think we should try to fix all incompatibilities that cause OOMs in SWFs that exist in the wild.

However, we can't postpone the release forever, and the conditions in this movie seem to be fairly rare. So let's throw in the towel on this one.

Bastiaan Jacques <bjacques>
Project Member
Mon 14 Mar 2011 09:07:32 PM UTC, comment #24:

I don't think this bug should be a blocker.
OOM conditions we can expect, being an interpreter.
What happens at that point is up to system.
Here I get an abort due to an exception being thrown,
which is safe behaviour to me.

This specific case seems to be about an incompatibility, but we have many incompatibilities and can't block on all of them.

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 09:34:56 AM UTC, comment #23:

Going back to the original movie, it looks like the inner loop is never entered. So the reduced testcase isn't replicating the exact thing being drawn by gnash :/ (ie: gnash still gets all reds).

For comparison, the pp enters the inner loop around 83 times.

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 09:19:21 AM UTC, comment #22:

Gnash finds a pixel with value 1387072 (one of the 6 allowed) 52628 times over 54469 non-reds (over 234496 total: 22.5%).

Never any 530725, 794414, 1584709, 1848141 or 1979986

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 09:15:26 AM UTC, comment #21:

The actual exit condition (in the inner loop) is that the
picked pixel is one of
530725, 794414, 1387072, 1584709, 1848141, 1979986

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 09:08:17 AM UTC, comment #20:

Anyway, the bitmap has 234496, most of which are red.
With pp ~ 27% of them are non-red (63558),
with gnash ~ 23% (54469)

Those are the probabilities of the inner loop to be entered.
The outer loop won't exit till the conditions in the inner
loop are satisfied too.

I guess next attempt will be to always enter the inner loop
and see if those conditions are ever met.

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 08:59:57 AM UTC, comment #19:

wait wait, I was wrong. Gnash wasn't drawing because it was called with -r0, which (I didn't expect that) changes behaviour of BitmapData.draw, it looks like

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 08:55:37 AM UTC, comment #18:

The attached testcase shows BitmapData.draw or BitmapData.getPixel doing something wrong. It draws character 181 from the eddie.swf movie (extracted) and calls getPixel on each pixel. Gnash finds NO pixel being not red, pp gets some reds.

Test can be improved to expect actual values

(file #22920)

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 08:45:36 AM UTC, comment #17:

Worth adding UNIMPL calls as first thing

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 08:10:42 AM UTC, comment #16:

That's a statically defined MovieClip in swf/eddie.swf; eddie.swf appears to do some tweening and other transformations, but most of the logic seems to be in main.swf once eddie.swf is loaded.

I also note that BitmapData.copyPixels, BitmapData.copyChannels, and BitmapData.applyFilter along with BitmapFilters are used and unimplemented.

Benjamin Wolsey <bwy>
Project Member
Mon 14 Mar 2011 08:09:54 AM UTC, comment #15:

_level0.baseMC.mainPaper.cachePaper0.eddie seems to be swf/eddie.swf, mc_24 is a sprite (character id 182) on which 4 unnamed objects are placed: 175, 177, 179, 181

Sandro Santilli <strk>
Project Member
Mon 14 Mar 2011 07:56:54 AM UTC, comment #14:

_level0.baseMC.mainPaper.cachePaper0.eddie.mc_24.instance18 is the MovieClip that causes the first loop.

Benjamin Wolsey <bwy>
Project Member
Mon 14 Mar 2011 07:14:52 AM UTC, comment #13:

The BitmapData constructor creates a bitmap the size of the MovieClip VectorMC and sets all pixels to red (0xff0000, 16711680). From Sandro's comments it seems that Gnash isn't drawing anything, or is at least drawing it wrong, because the pixels stay red afterwards too.

Reproducing it should be as easy as getting a definition of VectorMC and copying the BitmapData code, as any dynamic transformations of VectorMC should be ignored.

Anyway it seems that either VectorMC is somehow wrong, or BitmapData is.

Benjamin Wolsey <bwy>
Project Member
Sun 13 Mar 2011 10:09:47 PM UTC, comment #12:

I've modified main.swf to trace the return from getPixel.
pp gives many 16711680, even in big sequences, but it gives also other values sometimes.
Gnash, on the other hand, always output only 16711680.

Tracing x/y too I found a single pixel that shows it clearly:
pp:
11
34
1387072
Gnash:
TRACE: 11
TRACE: 34
TRACE: 16711680

Sandro Santilli <strk>
Project Member
Sun 13 Mar 2011 09:21:46 PM UTC, comment #11:

Oops, that's 11ab0d07c982c528f6106d334cfa40e9784f765e I've pushed (after pull --rebase)

Sandro Santilli <strk>
Project Member
Sun 13 Mar 2011 09:21:03 PM UTC, comment #10:

Ah, it's toString(16) for base 16, so number to string is fine. I've pushed the test in master (272b2d)

Sandro Santilli <strk>
Project Member
Sun 13 Mar 2011 09:14:36 PM UTC, comment #9:

Whether the loop would eventually finish I've no idea. If every pixel is red, it should never finish.

Sandro Santilli <strk>
Project Member
Sun 13 Mar 2011 08:48:57 PM UTC, comment #8:

Dumping actions in the loop shows every getPixel returning:
[number:1.67117e+07]

toString of that number returns [string:ff0000] (wonders!)
which added 0x to becomes [string:0xff0000] and get set to register 2.

So, pixel is red and the inner loop isn't entered.

This happens pretty much in every attempt, with randoms being:
70,13 65,7 51,29 5,47 68,31 40,25 56,32 57,46 23,45 30,24 23,20

btw, is it normal that [number:1.67117e+07].toString becomes ff0000 ?

Sandro Santilli <strk>
Project Member
Sun 13 Mar 2011 08:43:19 PM UTC, comment #7:

So the loop would eventually finish if you let it run without the script limits on a machine with lots of memory?

Bastiaan Jacques <bjacques>
Project Member
Sun 13 Mar 2011 08:25:55 PM UTC, comment #6:

The other thing to be noted is that the loop body creates Number and String temporaries, which will not be destroyed until the very end of the action block.

This is similar to the "potlatch eat all memory" issue, and could only be fixed by improving the memory management (ie: http://linuxfund.org/projects/gnash/)

Sandro Santilli <strk>
Project Member
Sun 13 Mar 2011 07:42:08 PM UTC, comment #5:

The loop seems to be this one (not confirmed):

do { /* 2nd type */
R3 = '0x'+R6.getPixel(random(R7),random(R8)).toString(16);
if( (R3!='0xff0000') ) {
R5 = this.darks.length;
while( (--R5>0) ) {
if( (Number(R3)==this.darks[R5]) ) {
vectorMC.colorID = R5;
R4 = false;
} else {
}
}
}
} while( R4);

Benjamin said:

Logically it depends on a pixel in a BitmapData not being red, and then
having its value contained in the array this.darks.

So the possibilities seem to be:

1. Number conversion is not working.
2. String conversion is not working (toString(16)).
3. this.darks contains incorrect values.
4. getPixel() is returning wrong values.
5. The BitmapData contains data that isn't expected (this is indeed
likely if it was produced using loadBitmap() or draw(), as Gnash's
values often differ slightly) and the code relies on it having a
particular value. It could also contain no data.

I don't consider the first two to be likely, so it would be useful to
know where the the 'darks' array comes from, and how the BitmapData is
produced.

R6 = new
flash.display.BitmapData(vectorMC._width,vectorMC._height,false,16711680);
R12 = new flash.geom.Transform(vectorMC);
R9 = new flash.geom.Matrix();
R10 = vectorMC.getBounds();
R9.translate((0-R10.xMin),(0-R10.yMin));
R6.draw(vectorMC,R9);

So it does use draw(), and darks is a global array of fixed values. This
may make it impossible to fix the infinite loop without getting agg to
render exactly like the pp, but it would be good to verify that
getPixel() is returning slightly different values from those expected.

Sandro Santilli <strk>
Project Member
Tue 08 Mar 2011 01:29:23 AM UTC, comment #4:

Seeing as this bug causes an abort on my machine, I guess it ought to block.

Bastiaan Jacques <bjacques>
Project Member
Tue 22 Feb 2011 07:21:03 AM UTC, comment #3:

For the record, entry point is bug32521/base.swf

Sandro Santilli <strk>
Project Member
Tue 22 Feb 2011 07:15:02 AM UTC, comment #2:

I've downloaded all files locally (many) for easier debugging.
Grab it while it's fresh (one day I'll remove from there)
http://strk.keybit.net/tmp/bug32521.tgz

Sandro Santilli <strk>
Project Member
Sat 19 Feb 2011 10:48:49 AM UTC, comment #1:

I can confirm this. The symptoms are that at unpredictable times a single ActionScript loop causes memory usage to rocket. This stops only when the execution time (hardcoded to 40 seconds) is exceeded. This is how we know it's an AS loop.

I observed memory usage first rising to 2.5 GB, then 4.5 GB in two such phases.

No memory leak is shown in valgrind, and the GC doesn't seem to have either a very large number or a significantly increasing number of objects.

I'm trying to see what the loop in question does using -va.

Benjamin Wolsey <bwy>
Project Member
Thu 17 Feb 2011 05:54:26 PM UTC, original submission:

http://www.thomasedison.org/base.swf

Play until the movie gets to the signature. The animation (and drawing) will stop. CPU usage will reach maximum eventually the OS will kill the process, presumably because of an OOM situation.

Bastiaan Jacques <bjacques>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #22920:  testcase.tgz added by strk (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by strk (Posted a comment)
  • -unavailable- added by bwy (Posted a comment)
  • -unavailable- added by bjacques (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 12 Sep 2011 12:16:35 PM UTCstrkStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Wed 30 Mar 2011 07:01:31 AM UTCstrkStatusConfirmed=>Ready For Test
    Sat 19 Mar 2011 04:06:24 PM UTCbjacquesSeverity4 - Important=>5 - Blocker
    Tue 15 Mar 2011 03:42:49 AM UTCbjacquesSeverity5 - Blocker=>4 - Important
    Mon 14 Mar 2011 08:55:37 AM UTCstrkAttached File-=>Added testcase.tgz, #22920
    Tue 08 Mar 2011 01:29:23 AM UTCbjacquesSeverity4 - Important=>5 - Blocker
    Sat 19 Feb 2011 10:48:49 AM UTCbwyStatusNone=>Confirmed
    Thu 17 Feb 2011 07:24:15 PM UTCbjacquesSummaryever-growing resource usage=>SIGKILL after ever-growing resource usage

    Back to the top


    Powered by Savane 3.1-cleanup1