Asan reports the following use-after-free after playing the attached PDF for a few seconds.

==24477==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000009688 at pc 0x7fbac034c593 bp 0x7fffa868ec00 sp 0x7fffa868ebf8
READ of size 8 at 0x619000009688 thread T0
    #0 0x7fbac034c592 in boost::intrusive_ptr<gnash::SWF::ControlTag>::operator->() const /usr/include/boost/smart_ptr/intrusive_ptr.hpp:162:9
    #1 0x7fbac0343a42 in gnash::MovieClip::executeFrameTags(unsigned long, gnash::DisplayList&, int) /home/bastiaan/gnash/libcore/MovieClip.cpp:1059:17
    #2 0x7fbac03432b5 in gnash::MovieClip::advance() /home/bastiaan/gnash/libcore/MovieClip.cpp:940:17
    #3 0x7fbac02f0805 in gnash::SWFMovie::advance() /home/bastiaan/gnash/libcore/SWFMovie.cpp:82:5
    #4 0x7fbac02fabd7 in gnash::movie_root::advanceLiveChars() /home/bastiaan/gnash/libcore/movie_root.cpp:2061:9
    #5 0x7fbac02fa225 in gnash::movie_root::advanceMovie() /home/bastiaan/gnash/libcore/movie_root.cpp:968:5
    #6 0x7fbac02f9e9a in gnash::movie_root::advance() /home/bastiaan/gnash/libcore/movie_root.cpp:933:17
    #7 0x7fbac0faeeaf in gnash::Gui::advanceMovie(bool) /home/bastiaan/gnash/gui/gui.cpp:954:27
    #8 0x7fbac0fe4769 in gnash::NullGui::run() /home/bastiaan/gnash/gui/NullGui.cpp:44:5
    #9 0x7fbac0fcad42 in gnash::Player::run(int, char**, std::string const&, std::string const&) /home/bastiaan/gnash/gui/Player.cpp:664:5
    #10 0x7fbac0f5d14c in playFile(gnash::Player&, int, char**, std::string const&) /home/bastiaan/gnash/gui/gnash.cpp:92:5
    #11 0x7fbac0f647c1 in void std::_Bind<void (*(std::reference_wrapper<gnash::Player>, int, char**, std::_Placeholder<1>))(gnash::Player&, int, char**, std::string const&)>::__call<void, std::string&, 0ul, 1ul, 2ul, 3ul>(std::tuple<std::string&>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /bin/../lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/functional:1263:11
    #12 0x7fbac0f64176 in void std::_Bind<void (*(std::reference_wrapper<gnash::Player>, int, char**, std::_Placeholder<1>))(gnash::Player&, int, char**, std::string const&)>::operator()<std::string&, void>(std::string&) /bin/../lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/functional:1321:11
    #13 0x7fbac0f63c12 in std::_Bind<void (*(std::reference_wrapper<gnash::Player>, int, char**, std::_Placeholder<1>))(gnash::Player&, int, char**, std::string const&)> std::for_each<__gnu_cxx::__normal_iterator<std::string*, std::vector<std::string, std::allocator<std::string> > >, std::_Bind<void (*(std::reference_wrapper<gnash::Player>, int, char**, std::_Placeholder<1>))(gnash::Player&, int, char**, std::string const&)> >(__gnu_cxx::__normal_iterator<std::string*, std::vector<std::string, std::allocator<std::string> > >, __gnu_cxx::__normal_iterator<std::string*, std::vector<std::string, std::allocator<std::string> > >, std::_Bind<void (*(std::reference_wrapper<gnash::Player>, int, char**, std::_Placeholder<1>))(gnash::Player&, int, char**, std::string const&)>) /bin/../lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/bits/stl_algo.h:3755:2
    #14 0x7fbac0f5d885 in main /home/bastiaan/gnash/gui/gnash.cpp:175:9
    #15 0x7fbabca43fdf in __libc_start_main (/lib64/libc.so.6+0x3629e1ffdf)
    #16 0x7fbac0f5cf7c in _start (/home/bastiaan/obj-gnash-clang-sanitize/gui/.libs/lt-gtk-gnash+0xf2f7c)

0x619000009688 is located 8 bytes inside of 1024-byte region [0x619000009680,0x619000009a80)
freed by thread T1 here:
    #0 0x7fbac0ed85ab in operator delete(void*) (/home/bastiaan/obj-gnash-clang-sanitize/gui/.libs/lt-gtk-gnash+0x6e5ab)
    #1 0x7fbac05a06a2 in void std::vector<boost::intrusive_ptr<gnash::SWF::ControlTag>, std::allocator<boost::intrusive_ptr<gnash::SWF::ControlTag> > >::_M_emplace_back_aux<boost::intrusive_ptr<gnash::SWF::ControlTag> const&>(boost::intrusive_ptr<gnash::SWF::ControlTag> const&) /bin/../lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/bits/vector.tcc:438:2
    #2 0x7fbac058ca71 in gnash::SWFMovieDefinition::addControlTag(boost::intrusive_ptr<gnash::SWF::ControlTag>) /home/bastiaan/gnash/libcore/parser/SWFMovieDefinition.h:274:9
    #3 0x7fbac0586387 in gnash::SWFMovieDefinition::addDisplayObject(unsigned short, gnash::SWF::DefinitionTag*) /home/bastiaan/gnash/libcore/parser/SWFMovieDefinition.cpp:163:5
    #4 0x7fbac05802fd in gnash::SWFParser::read(long) /home/bastiaan/gnash/libcore/parser/SWFParser.cpp:96:17
    #5 0x7fbac0585738 in gnash::SWFMovieDefinition::read_all_swf() /home/bastiaan/gnash/libcore/parser/SWFMovieDefinition.cpp:467:18
    #6 0x7fbabd0b2d9f (/lib64/libstdc++.so.6+0x3b79ebad9f)

previously allocated by thread T1 here:
    #0 0x7fbac0ed806b in operator new(unsigned long) (/home/bastiaan/obj-gnash-clang-sanitize/gui/.libs/lt-gtk-gnash+0x6e06b)
    #1 0x7fbac05a05b5 in void std::vector<boost::intrusive_ptr<gnash::SWF::ControlTag>, std::allocator<boost::intrusive_ptr<gnash::SWF::ControlTag> > >::_M_emplace_back_aux<boost::intrusive_ptr<gnash::SWF::ControlTag> const&>(boost::intrusive_ptr<gnash::SWF::ControlTag> const&) /bin/../lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/bits/vector.tcc:412:22
    #2 0x7fbac058ca71 in gnash::SWFMovieDefinition::addControlTag(boost::intrusive_ptr<gnash::SWF::ControlTag>) /home/bastiaan/gnash/libcore/parser/SWFMovieDefinition.h:274:9
    #3 0x7fbac03693f9 in gnash::SWF::DoInitActionTag::loader(gnash::SWFStream&, gnash::SWF::TagType, gnash::movie_definition&, gnash::RunResources const&) /home/bastiaan/gnash/libcore/swf/DoInitActionTag.h:96:9
    #4 0x7fbac05802fd in gnash::SWFParser::read(long) /home/bastiaan/gnash/libcore/parser/SWFParser.cpp:96:17
    #5 0x7fbac0585738 in gnash::SWFMovieDefinition::read_all_swf() /home/bastiaan/gnash/libcore/parser/SWFMovieDefinition.cpp:467:18
    #6 0x7fbabd0b2d9f (/lib64/libstdc++.so.6+0x3b79ebad9f)

Thread T1 created by T0 here:
    #0 0x7fbac0f2753f in pthread_create (/home/bastiaan/obj-gnash-clang-sanitize/gui/.libs/lt-gtk-gnash+0xbd53f)
    #1 0x7fbabd0b2ed8 in std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>) (/lib64/libstdc++.so.6+0x3b79ebaed8)

SUMMARY: AddressSanitizer: heap-use-after-free /usr/include/boost/smart_ptr/intrusive_ptr.hpp:162 boost::intrusive_ptr<gnash::SWF::ControlTag>::operator->() const
Shadow bytes around the buggy address:
  0x0c327fff9280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9290: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff92a0: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c327fff92b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fff92c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c327fff92d0: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff92e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff92f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff9300: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff9310: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff9320: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  ASan internal:           fe
==24477==ABORTING

So this clearly shows data deleted by the thread running read_all_swf() being accessed by another thread.

(file #32996)

I presume you mean MovieClip._framesloaded. But that says nothing about parsing, it only tells you when a frame is loaded, unless you assume that loading happens immediately after parsing. (Which might be true: I don't know, and I doubt it would be possible to be sure without disassembling the pp.) In any case, I think it's not that important to know when parsing happens; from the perspective of the SWF movie, presumably only loading is of importance.

How can you test at which rate the pp parses SWF data? I can see it would be straightforward to test when the data source is being read (querying getBytesLoaded might give a fair approximation), and at which rate, and even when the pp finishes loading certain things into the movie, but actually measuring parsing I can't figure out.

If the parsing is moved to the main thread, then the loaders  may have to be converted to use a push rather than pull model for the raw data, which could be a significant undertaking. Alternatively, some sort of threaded loading could be built into SWFStream itself, which I expect would be simpler.

As an alternative to moving parsing into the main thread, the loaders could be modified to return their parsing results (rather than modifying SWFMovieDefinition directly). Then the parsing results would have to be marshalled back to the main thread regularly.

In either case, there would have to be some point at which the either the parsing is actually done or the parsing results are loaded into the main thread, which owns SWFMovieDefinition. Presumably this could happen in ensure_frame_loaded().

It seems obvious to me that reading from SWFStream (which typically involves downloading stuff) should be asynchronous, but I'm not so sure about the parsing done in the loaders. I think we might be able to get away with doing that synchronously, but I'm not sure.

What do you think?

The more elegant solution would be to stop using threads for loading and use async IO instead...

read_all_swf() is normally called from a separate thread. It parses the SWF and calls tag loaders for each tag encounters. The tag loader functions receive a reference to 'this' (SWFMovieDefinition object). The trouble is that the loaders call a host of different functions from the SWFMovieDefinition object, some of which are access data members that are not mutex-locked. A quick survey of the functions called are:

Mutex-protected:
  addDisplayObject
  getDefinitionTag

Not mutex-protected:
  addControlTag*
  add_font
  get_font
  get_sound_sample
  getBitmap
  get_loading_sound_stream_id

• addControlTag actually had a partial mutex protection for the data members access there, by being used in conjunction with _frames_loaded, which had its own mutex; although the access of m_playlist in read_all_swf was unprotected. In any case, the _frames_load mutex was removed in 869f2fa5be27.

Of these, add_font and addControlTag are the most serious, as data is written by the thread to class members in those functions. I'm not sure of the impact of the lack of mutex protection of the members read in the other functions.

See also bug #32569.