[Gnash-commit] gnash ChangeLog server/asobj/NetStream.cpp serv...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  07/06/01 11:53:19

Modified files:
        .              : ChangeLog 
        server/asobj   : NetStream.cpp NetStream.h 

Log message:
                * server/asobj/NetStream.{cpp,h}: split 
processStatusNotifications
                  so that onStatus handler call is performed while the 
statusMutex
                  is NOT locked, to prevent attempts to lock the mutex again 
while
                  reacting to status handlers. statusQueue type changed from 
vector
                  to deque, for faster pop_front.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.3433&r2=1.3434
http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/NetStream.cpp?cvsroot=gnash&r1=1.61&r2=1.62
http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/NetStream.h?cvsroot=gnash&r1=1.44&r2=1.45

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.3433
retrieving revision 1.3434
diff -u -b -r1.3433 -r1.3434
--- ChangeLog   1 Jun 2007 11:02:16 -0000       1.3433
+++ ChangeLog   1 Jun 2007 11:53:18 -0000       1.3434
@@ -1,5 +1,13 @@
 2007-06-01 Sandro Santilli <address@hidden>
 
+       * server/asobj/NetStream.{cpp,h}: split processStatusNotifications
+         so that onStatus handler call is performed while the statusMutex
+         is NOT locked, to prevent attempts to lock the mutex again while
+         reacting to status handlers. statusQueue type changed from vector
+         to deque, for faster pop_front.
+
+2007-06-01 Sandro Santilli <address@hidden>
+
        * gui/Player.cpp (run): remove sound_handler cleanup, will
          be done by gnash::clear().
        * gui/: gtk.cpp, gtksup.h: implement gui::quit()

Index: server/asobj/NetStream.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStream.cpp,v
retrieving revision 1.61
retrieving revision 1.62
diff -u -b -r1.61 -r1.62
--- server/asobj/NetStream.cpp  31 May 2007 15:52:28 -0000      1.61
+++ server/asobj/NetStream.cpp  1 Jun 2007 11:53:19 -0000       1.62
@@ -17,7 +17,7 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
-/* $Id: NetStream.cpp,v 1.61 2007/05/31 15:52:28 tgc Exp $ */
+/* $Id: NetStream.cpp,v 1.62 2007/06/01 11:53:19 strk Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -453,23 +453,19 @@
 void
 NetStream::processStatusNotifications()
 {
-       // Get an exclusive lock so any notification from loader thread will 
wait
-       boost::mutex::scoped_lock lock(statusMutex);
-
-       // No queued statuses to notify ...
-       if ( _statusQueue.empty() ) return;
-
        // TODO: check for System.onStatus too ! use a private 
getStatusHandler() method for this.
        as_value status;
-       if ( get_member("onStatus", &status) && status.is_function())
+       if ( ! get_member("onStatus", &status) || ! status.is_function())
        {
-#ifdef GNASH_DEBUG_STATUS
-               log_debug("Processing "SIZET_FMT" status notifications", 
_statusQueue.size());
-#endif
+               clearStatusQueue();
+               return;
+       }
 
-               for (StatusQueue::iterator it=_statusQueue.begin(), 
itE=_statusQueue.end(); it!=itE; ++it)
+       StatusCode code;
+       while (1)
                {
-                       StatusCode code = *it; 
+               code = popNextPendingStatusNotification();
+               if ( code == invalidStatus ) break; // no more pending 
notifications
 
 #ifdef GNASH_DEBUG_STATUS
                        log_debug(" Invoking onStatus(%s)", 
getStatusCodeInfo(code).first);
@@ -482,21 +478,18 @@
                        call_method(status, m_env, this, 1, 
m_env->get_top_index() );
                }
 
-       }
-
-       _statusQueue.clear();
 
 }
 
 void
 NetStream::setStatus(StatusCode status)
 {
-       // status unchanged
-       if ( _lastStatus == status) return;
-
        // Get a lock to avoid messing with statuses while processing them
        boost::mutex::scoped_lock lock(statusMutex);
 
+       // status unchanged
+       if ( _lastStatus == status) return;
+
        _lastStatus = status;
        _statusQueue.push_back(status);
 }
@@ -611,5 +604,28 @@
        return o;
 }
 
+NetStream::StatusCode
+NetStream::popNextPendingStatusNotification()
+{
+       // Get an exclusive lock on the queue
+       boost::mutex::scoped_lock lock(statusMutex);
+
+       // No queued statuses to notify ...
+       if ( _statusQueue.empty() ) return invalidStatus;
+
+       StatusCode nextCode = _statusQueue.front();
+       _statusQueue.pop_front();
+       return nextCode;
+}
+
+void
+NetStream::clearStatusQueue()
+{
+       // Get an exclusive lock on the queue
+       boost::mutex::scoped_lock lock(statusMutex);
+
+       _statusQueue.clear();
+}
+
 
 } // end of gnash namespace

Index: server/asobj/NetStream.h
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStream.h,v
retrieving revision 1.44
retrieving revision 1.45
diff -u -b -r1.44 -r1.45
--- server/asobj/NetStream.h    31 May 2007 15:52:28 -0000      1.44
+++ server/asobj/NetStream.h    1 Jun 2007 11:53:19 -0000       1.45
@@ -19,7 +19,7 @@
 //
 //
 
-/*  $Id: NetStream.h,v 1.44 2007/05/31 15:52:28 tgc Exp $ */
+/*  $Id: NetStream.h,v 1.45 2007/06/01 11:53:19 strk Exp $ */
 
 #ifndef __NETSTREAM_H__
 #define __NETSTREAM_H__
@@ -37,6 +37,8 @@
 #include "NetConnection.h"
 #include "FLVParser.h"
 
+#include <deque>
+
 // Forward declarations
 namespace gnash {
        //class NetConnection;
@@ -103,12 +105,20 @@
        ///  - NetStream.Play.StreamNotFound
        ///  - NetStream.Seek.InvalidTime
        ///
+       /// This method locks the statusMutex during operations
+       ///
        void setStatus(StatusCode code);
 
        /// \brief
        /// Call any onStatus event handler passing it
        /// any queued status change, see _statusQueue
        //
+       /// Will NOT lock the statusMutex itself, rather it will
+       /// iteratively call the popNextPendingStatusNotification()
+       /// private method, which will take care of locking it.
+       /// This is to make sure onStatus handler won't call methods
+       /// possibly trying to obtain the lock again (::play, ::pause, ...)
+       ///
        void processStatusNotifications();
 
        // The actionscript enviroment for the AS callbacks
@@ -266,7 +276,23 @@
 
 private:
 
-       typedef std::vector<StatusCode> StatusQueue;
+       /// Pop next queued status notification from the queue
+       //
+       /// Lock the statusMutex during operations
+       ///
+       /// @return The status code to notify, or invalidStatus when
+       ///         the queue is empty
+       ///
+       StatusCode popNextPendingStatusNotification();
+
+       /// Clear status notification queue
+       //
+       /// Lock the statusMutex during operations
+       ///
+       void clearStatusQueue();
+
+       // TODO: change to a container with fast pop_front()
+       typedef std::deque<StatusCode> StatusQueue;
 
        /// List of status messages to be processed
        StatusQueue _statusQueue;