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

 
 

bug #32521: SIGKILL after ever-growing resource usage

Submitter:  Bastiaan Jacques <bjacques>
Submitted:  Thu 17 Feb 2011 05:54:26 PM UTC
   
 
Category:  ActionScript Severity:  5 - Blocker
Release:  master Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

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

this is clearly fixed.

Sandro Santilli <strk>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group Member
Mon 14 Mar 2011 08:45:36 AM UTC, comment #17: 

Worth adding UNIMPL calls as first thing

Sandro Santilli <strk>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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>
Group Member
Tue 22 Feb 2011 07:21:03 AM UTC, comment #3: 

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

Sandro Santilli <strk>
Group 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>
Group 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>
Group 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>
Group Member

 

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

Attach Files:
   
   
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
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by bwy (Posted a comment)
  • -email is unavailable- added by bjacques (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-09-12 strk StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2011-03-30 strk StatusConfirmed Ready For Test
    2011-03-19 bjacques Severity4 - Important 5 - Blocker
    2011-03-15 bjacques Severity5 - Blocker 4 - Important
    2011-03-14 strk Attached File- Added testcase.tgz, #22920
    2011-03-08 bjacques Severity4 - Important 5 - Blocker
    2011-02-19 bwy StatusNone Confirmed
    2011-02-17 bjacques Summaryever-growing resource usage SIGKILL after ever-growing resource usage

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code