[Gnash-commit] gnash ChangeLog server/parser/movie_def_impl.cp...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  07/11/26 12:56:34

Modified files:
        .              : ChangeLog 
        server/parser  : movie_def_impl.cpp movie_def_impl.h 

Log message:
         consistent locking for _frames_loaded (so helgrind is happy).

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.4965&r2=1.4966
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/movie_def_impl.cpp?cvsroot=gnash&r1=1.89&r2=1.90
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/movie_def_impl.h?cvsroot=gnash&r1=1.59&r2=1.60

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.4965
retrieving revision 1.4966
diff -u -b -r1.4965 -r1.4966
--- ChangeLog   26 Nov 2007 11:33:17 -0000      1.4965
+++ ChangeLog   26 Nov 2007 12:56:34 -0000      1.4966
@@ -1,5 +1,7 @@
 2007-11-26 Sandro Santilli <address@hidden>
 
+       * server/parser/movie_def_impl.{cpp,h}: consistent locking for
+         _frames_loaded (so helgrind is happy).
        * gui/: Makefile.am, gnash.cpp: add CXXFLAGS in --version output.
 
 2007-11-26 Zou Lunkai <address@hidden>

Index: server/parser/movie_def_impl.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/parser/movie_def_impl.cpp,v
retrieving revision 1.89
retrieving revision 1.90
diff -u -b -r1.89 -r1.90
--- server/parser/movie_def_impl.cpp    23 Nov 2007 12:21:26 -0000      1.89
+++ server/parser/movie_def_impl.cpp    26 Nov 2007 12:56:34 -0000      1.90
@@ -548,12 +548,12 @@
 bool
 movie_def_impl::ensure_frame_loaded(size_t framenum)
 {
+       boost::mutex::scoped_lock lock(_frames_loaded_mutex);
+
 #ifndef LOAD_MOVIES_IN_A_SEPARATE_THREAD
        return ( framenum <= _frames_loaded );
 #endif
 
-       boost::mutex::scoped_lock lock(_frames_loaded_mutex);
-
        if ( framenum <= _frames_loaded ) return true;
 
        _waiting_for_frame = framenum;
@@ -872,8 +872,8 @@
                                log_parse("  show_frame");
                        );
 
-                       incrementLoadedFrames();
-                       if ( _frames_loaded == m_frame_count )
+                       size_t floaded = incrementLoadedFrames();
+                       if ( floaded == m_frame_count )
                        {
                                str.close_tag();
                                tag_type = str.open_tag();
@@ -923,23 +923,23 @@
                log_error(_("Parsing exception: %s"), e.what());
        }
 
-       if ( ! m_playlist[_frames_loaded].empty() )
+       size_t floaded = get_loading_frame();
+       if ( ! m_playlist[floaded].empty() )
        {
                IF_VERBOSE_MALFORMED_SWF(
                log_swferror(_(SIZET_FMT " control tags are NOT followed by"
                        " a SHOWFRAME tag"),
-                       m_playlist[_frames_loaded].size());
+                       m_playlist[floaded].size());
                );
        }
 
-       if ( m_frame_count > _frames_loaded )
+       if ( m_frame_count > floaded )
        {
                IF_VERBOSE_MALFORMED_SWF(
                log_swferror(_(SIZET_FMT " frames advertised in header, but 
only " SIZET_FMT " SHOWFRAME tags "
-                       "found in stream. Updating total frames count"), 
m_frame_count, _frames_loaded);
+                       "found in stream. Updating total frames count"), 
m_frame_count, floaded);
                );
-               boost::mutex::scoped_lock lock(_frames_loaded_mutex);
-               m_frame_count = _frames_loaded;
+               m_frame_count = floaded;
                // Notify any thread waiting on frame reached condition
                _frame_reached_condition.notify_all();
        }
@@ -952,7 +952,7 @@
        return _frames_loaded;
 }
 
-void
+size_t
 movie_def_impl::incrementLoadedFrames()
 {
        boost::mutex::scoped_lock lock(_frames_loaded_mutex);
@@ -989,6 +989,8 @@
                // See: http://boost.org/doc/html/condition.html
                _frame_reached_condition.notify_all();
        }
+
+       return _frames_loaded;
 }
 
 void
@@ -1096,7 +1098,8 @@
 void
 movie_def_impl::add_frame_name(const std::string& n)
 {
-       boost::mutex::scoped_lock lock(_namedFramesMutex);
+       boost::mutex::scoped_lock lock1(_namedFramesMutex);
+       boost::mutex::scoped_lock lock2(_frames_loaded_mutex);
        //log_debug(_("labelframe: frame %d, name %s"), _frames_loaded, 
n.c_str());
        _namedFrames[n] = _frames_loaded;
 }

Index: server/parser/movie_def_impl.h
===================================================================
RCS file: /sources/gnash/gnash/server/parser/movie_def_impl.h,v
retrieving revision 1.59
retrieving revision 1.60
diff -u -b -r1.59 -r1.60
--- server/parser/movie_def_impl.h      23 Nov 2007 12:21:26 -0000      1.59
+++ server/parser/movie_def_impl.h      26 Nov 2007 12:56:34 -0000      1.60
@@ -306,7 +306,9 @@
        ///
        /// NOTE: this method locks _frames_loaded_mutex
        ///
-       void incrementLoadedFrames();
+       /// @return the new value of _frames_loaded
+       ///
+       size_t incrementLoadedFrames();
 
        /// Set number of bytes loaded from input stream
        //
@@ -485,6 +487,7 @@
        void    addControlTag(ControlTag* tag)
        {
            assert(tag);
+           boost::mutex::scoped_lock lock(_frames_loaded_mutex);
            m_playlist[_frames_loaded].push_back(tag);
        }
 
@@ -494,7 +497,7 @@
 
        // See dox in movie_definition.h
        //
-       // locks _namedFramesMutex
+       // locks _namedFramesMutex and _frames_loaded_mutex
        //
        void add_frame_name(const std::string& name);
 
@@ -514,7 +517,10 @@
 
        virtual const PlayList* getPlaylist(size_t frame_number) const
        {
+#ifndef NDEBUG
+               boost::mutex::scoped_lock lock(_frames_loaded_mutex);
                assert(frame_number <= _frames_loaded);
+#endif
 
                PlayListMap::const_iterator it = m_playlist.find(frame_number);
                if ( it == m_playlist.end() ) return NULL;