bugGnash - The GNU Flash player - Bugs: bug #22982, Segfault in advanceLiveChar

 
 

bug #22982: Segfault in advanceLiveChar

Submitter:  Benjamin Wolsey <bwy>
Submitted:  Sat 19 Apr 2008 06:55:36 PM UTC
   
 
Category:  core Severity:  6 - Security
Release:  None Status:  Fixed
Privacy:  Public Assigned to:  strk
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 25 Apr 2008 10:18:15 AM UTC, comment #24: 

Well, we fixed the original bug anyway, so I'm closing this.

Sandro Santilli <strk>
Group Member
Fri 25 Apr 2008 09:14:07 AM UTC, comment #23: 

In step2, GC doesn't find childB as reachable, unless it was marked reachable by parentA.

Sandro Santilli <strk>
Group Member
Fri 25 Apr 2008 09:01:57 AM UTC, comment #22: 


> we may still reproduce the bug by having an onUnload event
> handler call a gotoFrame or similar thing triggering placement
> of new childs that will never get removed, so segfaults
> or memleaks are still pending till we work on a general
> fix for the issue


Just reviewed the code, it's fine for an unloaded character to add some new childs for itself. The new childs won't be leaked if I understand correctly.

eg.
step1. parentA which was marked as unloaded creates a normal childB(by ActionScripts or tags).
step2. GC scans. Scans childB, it's alive, we don't touch it. Scans parentA, it's unloaded, we destroy it. At this step, we also destroys childB, since destroying a character should destroy all its childs.
step3. GC finishes. parentA is cleared from the memory, childB is also cleared.

any other cases I haven't thought about?



Zou Lunkai <zoulunkai>
Thu 24 Apr 2008 12:56:45 AM UTC, comment #21: 

I already did some tests. I understand what's the problem we have. Just think that we don't need a hurry fix for the "adding new child in an unload handler", it's not urgent. I'll write a section about how to maintain characters life time on the wiki page first, then follow some testfiles, then(hopefully) a general fix.

I am still working on the displaylist tags today.

Zou Lunkai <zoulunkai>
Wed 23 Apr 2008 03:19:45 PM UTC, comment #20: 

No more assertion failures now.
zou: I guess you can still handle to trigger failures if you can create childs (and call stagePlacementCallback on them) while unloaded.

Sandro Santilli <strk>
Group Member
Wed 23 Apr 2008 12:20:28 PM UTC, comment #19: 

building with PARANOIA_LEVEL=2 trigger a lot of aborts in our testsuite, interesting ...

Sandro Santilli <strk>
Group Member
Wed 23 Apr 2008 07:05:11 AM UTC, comment #18: 


> (1)Unloaded characters could use createEmptyMovieClip() to add new child.


Do you get an assertion failure in this case when building
gnash with -DPARANOIA_LEVEL=2 ?

I think you would (to prevent the newly created
movieclip from leaking)

Did you check if the same applies for "destroyed" characters ?

Sandro Santilli <strk>
Group Member
Wed 23 Apr 2008 01:49:47 AM UTC, comment #17: 


> "Unloaded character should never allow modification their
> state, and in particular, should never place anything on
> stage"


Tested in an unload event handler:

(1)Unloaded characters could use createEmptyMovieClip() to add new child.
(2)Unloaded characters won't trigger executing any of its own frame tags.

I'll do some more tests to see if there is a general rule.


Zou Lunkai <zoulunkai>
Tue 22 Apr 2008 01:13:00 PM UTC, comment #16: 

I committed the ignore of events for unloaded buttons.
Could not produce a testcase showing any onUnload handler
should be triggered... Still interested in similar
failures with sprites, HINT: add -DGNASH_PARANOIA_LEVEL=2
in your CXXFLAGS

Sandro Santilli <strk>
Group Member
Tue 22 Apr 2008 11:55:40 AM UTC, comment #15: 

So, the problem is that a button gets' called on_button_event
against after being unloaded.
The event handler, in turn, changes the button's active childs
instantiating new characters and never getting a chance to
unload them again (self being already unloaded).

One fix is not responding to events when unloaded, for a button.
But a similar issue is still pending for sprites or any other
character on which events are sent while unloaded.

In general, an unloaded character should likely not react to
load events, but since UNLOAD itself is such an event, we
may still reproduce the bug by having an onUnload event
handler call a gotoFrame or similar thing triggering placement
of new childs that will never get removed, so segfaults
or memleaks are still pending till we work on a general
fix for the issue, which is likely:

"Unloaded character should never allow modification their state,
and in particular, should never place anything on stage"

Sandro Santilli <strk>
Group Member
Tue 22 Apr 2008 09:42:10 AM UTC, comment #14: 

testsuite/misc-swfc.all/button_test2.swf can now be used to reproduce the problem. There's still no automatic runner,
see the .sc file for instructions on how to reproduce.

Sandro Santilli <strk>
Group Member
Tue 22 Apr 2008 08:23:02 AM UTC, comment #13: 

While marking reachable resources in _liveChars fixes the segfault,
you can see the _liveChars container growing as you move the mouse
over and out the active button area.
So the bug is really in a missing ::unload (or ::destroy) call
somewhere.

Sandro Santilli <strk>
Group Member
Mon 21 Apr 2008 02:32:04 PM UTC, comment #12: 

Even more reduced version.

(file #15494)

Benjamin Wolsey <bwy>
Group Member
Mon 21 Apr 2008 02:23:11 PM UTC, comment #11: 

Attaching reduced version of testcase.

(file #15493)

Benjamin Wolsey <bwy>
Group Member
Mon 21 Apr 2008 01:57:42 PM UTC, comment #10: 

Marking live chars in movie_root fixes this case.
This is why it's not done:

    // NOTE: we don't need to mark _liveChars as any elements in that list
    //       should be NOT unloaded and thus marked as reachable by their
    //       parent.
    std::for_each(_liveChars.begin(), _liveChars.end(), boost::bind(&character::setReachable, _1));

Since it fails that "assumption" there must be some character
which is NOT marked as unloaded but still NOT reachable trough
parent.

Sandro Santilli <strk>
Group Member
Mon 21 Apr 2008 01:11:00 PM UTC, comment #9: 

Interestingly 0.8.2 doesn't trigger the segfault, and
gives clean valgrind reports.

Sandro Santilli <strk>
Group Member
Sun 20 Apr 2008 07:52:11 PM UTC, comment #8: 

Seems really a failed reachable scan.
It's fixed by skipping the SWEEP scan.
Valgrind says:

==28615== Invalid read of size 1
==28615==    at 0x42A679B: gnash::movie_root::advanceLiveChar(boost::intrusive_ptr<gnash::character>) (movie_root.cpp:1759)
==28615==    by 0x42C9522: boost::_bi::bind_t<void, void ()(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> ()()> > std::for_each<__
gnu_debug::_Safe_iterator<__gnu_norm::_List_iterator<boost::intrusive_ptr<gnash::character> >, __gnu_debug_def::list<boost::intrusive_ptr<gnash::character>, st
d::allocator<boost::intrusive_ptr<gnash::character> > > >, boost::_bi::bind_t<void, void (*)(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::
arg<1> (*)()> > >(__gnu_debug::_Safe_iterator<__gnu_norm::_List_iterator<boost::intrusive_ptr<gnash::character> >, __gnu_debug_def::list<boost::intrusive_ptr<g
nash::character>, std::allocator<boost::intrusive_ptr<gnash::character> > > >, __gnu_debug::_Safe_iterator<__gnu_norm::_List_iterator<boost::intrusive_ptr<gnas
h::character> >, __gnu_debug_def::list<boost::intrusive_ptr<gnash::character>, std::allocator<boost::intrusive_ptr<gnash::character> > > >, boost::_bi::bind_t<
void, void ()(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> ()()> >) (bind.hpp:231)
==28615==    by 0x42B5149: gnash::movie_root::advanceLiveChars() (movie_root.cpp:1781)
==28615==    by 0x42B6CDA: gnash::movie_root::advance() (movie_root.cpp:1032)
==28615==    by 0x809DC66: gnash::Gui::advanceMovie() (gui.cpp:771)
==28615==    by 0x80C1810: gnash::Gui::advance_movie(gnash::Gui*) (gui.h:236)
==28615==    by 0x4C708D5: (within /usr/lib/libglib-2.0.so.0.1400.1)
==28615==    by 0x4C7011B: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.1400.1)
==28615==    by 0x4C7355E: (within /usr/lib/libglib-2.0.so.0.1400.1)
==28615==    by 0x4C73908: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.1400.1)
==28615==    by 0x49369E3: gtk_main (in /usr/lib/libgtk-x11-2.0.so.0.1200.0)
==28615==    by 0x80BBC9D: gnash::GtkGui::run() (gtk.cpp:227)
==28615==  Address 0x71db6b0 is 200 bytes inside a block of size 516 free'd
==28615==    at 0x40222EC: operator delete(void*) (vg_replace_malloc.c:342)
==28615==    by 0x42EC4B4: gnash::sprite_instance::~sprite_instance() (sprite_instance.cpp:2264)
==28615==    by 0x4787F7D: gnash::GC::cleanUnreachable() (GC.cpp:93)

Sandro Santilli <strk>
Group Member
Sat 19 Apr 2008 09:27:19 PM UTC, comment #7: 

Even better: http://atlas.tagesschau.de/menue6.swf
That one segfaults run locally

Sandro Santilli <strk>
Group Member
Sat 19 Apr 2008 09:25:28 PM UTC, comment #6: 

Another run tip: you don't need the mode=country part, so
can download it:

gnash -v -r1 -d500 index12.swf -U http://atlas.tagesschau.de

Sandro Santilli <strk>
Group Member
Sat 19 Apr 2008 09:19:36 PM UTC, comment #5: 

Another thing to not (unrelated with the segfault) is
the movie triggers creation of many fonts that should
likely be the same one instead:
3061847744] 23:17:50 DEBUG: Initialized device font face 'Verdana' bold
3061847744] 23:17:50 DEBUG: Initialized device font face 'Verdana' bold
3061847744] 23:17:50 DEBUG: Initialized device font face 'Verdana' bold

Sandro Santilli <strk>
Group Member
Sat 19 Apr 2008 09:17:59 PM UTC, comment #4: 

Yep, could reproduce.

The most costly operation seems to be moving the mouse around.
Still segfaults with -r1.

The operation opening the pull-down menu seems to
be a user-defined event on a button (onRelease).
Gnash executes it immediately rather then push
on the queue.

Sandro Santilli <strk>
Group Member
Sat 19 Apr 2008 08:08:15 PM UTC, comment #3: 

Finally... That took about half-an-hour. And it's pretty much just the same thing.

==3730== Invalid read of size 1
==3730==    at 0x5399EE3: gnash::movie_root::advanceLiveChar(boost::intrusive_ptr<gnash::character>) (movie_root.cpp:1759)
==3730==    by 0x53A106F: boost::_bi::bind_t<void, void ()(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > > std::for_each<std::_List_iterator<boost::intrusive_ptr<gnash::character> >, boost::_bi::bind_t<void, void ()(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > > >(std::_List_iterator<boost::intrusive_ptr<gnash::character> >, std::_List_iterator<boost::intrusive_ptr<gnash::character> >, boost::_bi::bind_t<void, void (*)(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > >) (bind.hpp:230)
==3730==    by 0x5399ED9: gnash::movie_root::advanceLiveChars() (movie_root.cpp:1781)
==3730==    by 0x53A0785: gnash::movie_root::advance() (movie_root.cpp:1032)
==3730==    by 0x42B7FD: gnash::Gui::advanceMovie() (gui.cpp:768)
==3730==    by 0x43CDD8: gnash::Gui::advance_movie(gnash::Gui*) (gui.h:236)
==3730==    by 0x69A137A: (within /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x69A0C23: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x69A3F1C: (within /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x69A4216: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x5E19342: gtk_main (in /usr/lib64/libgtk-x11-2.0.so.0.1200.1)
==3730==    by 0x4389E5: gnash::GtkGui::run() (gtk.cpp:227)
==3730==  Address 0x12406540 is not stack'd, malloc'd or (recently) free'd
==3730==
==3730== Invalid read of size 8
==3730==    at 0x5399EF0: gnash::movie_root::advanceLiveChar(boost::intrusive_ptr<gnash::character>) (movie_root.cpp:1764)
==3730==    by 0x53A106F: boost::_bi::bind_t<void, void ()(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > > std::for_each<std::_List_iterator<boost::intrusive_ptr<gnash::character> >, boost::_bi::bind_t<void, void ()(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > > >(std::_List_iterator<boost::intrusive_ptr<gnash::character> >, std::_List_iterator<boost::intrusive_ptr<gnash::character> >, boost::_bi::bind_t<void, void (*)(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > >) (bind.hpp:230)
==3730==    by 0x5399ED9: gnash::movie_root::advanceLiveChars() (movie_root.cpp:1781)
==3730==    by 0x53A0785: gnash::movie_root::advance() (movie_root.cpp:1032)
==3730==    by 0x42B7FD: gnash::Gui::advanceMovie() (gui.cpp:768)
==3730==    by 0x43CDD8: gnash::Gui::advance_movie(gnash::Gui*) (gui.h:236)
==3730==    by 0x69A137A: (within /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x69A0C23: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x69A3F1C: (within /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x69A4216: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.1400.1)
==3730==    by 0x5E19342: gtk_main (in /usr/lib64/libgtk-x11-2.0.so.0.1200.1)
==3730==    by 0x4389E5: gnash::GtkGui::run() (gtk.cpp:227)
==3730==  Address 0x12406448 is not stack'd, malloc'd or (recently) free'd
==3730==
==3730== Jump to the invalid address stated on the next line
==3730==    at 0x68E7: ???
==3730==  Address 0x68E7 is not stack'd, malloc'd or (recently) free'd
==3730==
==3730== Process terminating with default action of signal 11 (SIGSEGV)
==3730==  Bad permissions for mapped region at address 0x68E7
==3730==    at 0x68E7: ???

Benjamin Wolsey <bwy>
Group Member
Sat 19 Apr 2008 08:00:34 PM UTC, comment #2: 

Running that in valgrind is torture - I just had to kill everything.

I can reproduce it every time. Precisely like this:

Run the movie and wait for the 'news' symbols to load.

Click on 'Kontinente' and wait for the menu to appear.

Click on 'Kontinente' again and wait for the menu to disappear.

Hover over 'Nachrichten' (or click on it).

It's only after you also close a menu that the segfault arises.

Benjamin Wolsey <bwy>
Group Member
Sat 19 Apr 2008 07:47:18 PM UTC, comment #1: 

It seems I can't reproduce it.
Can you reproduce it every time ?
Is there anything making it disappear ?
-r1 ?
#undef LOAD_MOVIES_IN_SEPARATE_THREAD ?

What flags are you building with ?

Sandro Santilli <strk>
Group Member
Sat 19 Apr 2008 06:55:36 PM UTC, original submission:  

This movie (again):

gnash "http://atlas.tagesschau.de/index12.swf?modus=&country=" -U http://atlas.tagesschau.de

gives a segfault. I get it by clicking on 'Kontinente' and on 'Nachrichten' (buttons top left) in any order. It takes a while to parse and load everything, so you have to wait until the menus appear.

Backtrace:

#1  0x00002b5784a5e070 in std::for_each<std::_List_iterator<boost::intrusive_ptr<gnash::character> >, boost::_bi::bind_t<void, void (*)(boost::intrusive_ptr<gnash::character>), boost::_bi::list1<boost::arg<1> > > > (
    __first=<value optimized out>, __last={_M_node = 0xa5f480}, __f=
        {f_ = 0x2b5784a56ee0 <gnash::movie_root::advanceLiveChar(boost::intrusive_ptr<gnash::character>)>, l_ = {a1_ = {<No data fields>}}})
    at /usr/include/boost/bind.hpp:230
#2  0x00002b5784a56eda in gnash::movie_root::advanceLiveChars (this=0xa5f470)
    at movie_root.cpp:1781
#3  0x00002b5784a5d786 in gnash::movie_root::advance (this=0xa5f470)
    at movie_root.cpp:1032
#4  0x000000000042b7fe in gnash::Gui::advanceMovie (this=0x7e40b0)
    at gui.cpp:768
#5  0x000000000043cdd9 in gnash::Gui::advance_movie (gui=0x248acf0)
    at gui.h:236

Benjamin Wolsey <bwy>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #15494:  smaller_segf.swf added by bwy (176B - application/x-shockwave-flash)
file #15493:  segf.swf added by bwy (225B - application/x-shockwave-flash - segfaulting testcase)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by zoulunkai (Posted a comment)
  • -email is unavailable- added by strk
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by bwy (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-04-25 strk StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2008-04-24 zoulunkai CategoryNone core
    2008-04-22 strk StatusIn Progress Ready For Test
    2008-04-22 strk StatusConfirmed In Progress
        Assigned toNone strk
        Carbon-Copy- Added zoulunkai
    2008-04-21 bwy Attached File- Added smaller_segf.swf, #15494
    2008-04-21 bwy Attached File- Added segf.swf, #15493
    2008-04-19 strk StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code