From 4bf3adffb8d58a7a856307df3460ac3eb4bf6d91 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Tue, 6 Sep 2011 22:26:28 +0100
Subject: In SceneViewer, introduce an IsEnabled flag and perform Close() under
 an m_pendingObjects lock in order to avoid the race condition seen by
 danbanner in http://opensimulator.org/mantis/view.php?id=5669

---
 .../Region/Framework/Interfaces/ISceneViewer.cs    | 11 +++-
 OpenSim/Region/Framework/Scenes/SceneViewer.cs     | 59 +++++++++++++++-------
 2 files changed, 51 insertions(+), 19 deletions(-)

(limited to 'OpenSim/Region')

diff --git a/OpenSim/Region/Framework/Interfaces/ISceneViewer.cs b/OpenSim/Region/Framework/Interfaces/ISceneViewer.cs
index 2397f22..a4cc2be 100644
--- a/OpenSim/Region/Framework/Interfaces/ISceneViewer.cs
+++ b/OpenSim/Region/Framework/Interfaces/ISceneViewer.cs
@@ -30,12 +30,21 @@ using OpenSim.Region.Framework.Scenes;
 
 namespace OpenSim.Region.Framework.Interfaces
 {
+    /// <summary>
+    /// Sends scheduled updates to it's associated ScenePresence.
+    /// </summary>
     public interface ISceneViewer
     {
         void Reset();
         void Close();
+
+        /// <summary>
+        /// Add the part to the queue of parts for which we need to send an update to the client
+        /// </summary>
+        /// <param name="part"></param>
         void QueuePartForUpdate(SceneObjectPart part);
+
         void SendPrimUpdates();
         int GetPendingObjectsCount();
     }
-}
+}
\ No newline at end of file
diff --git a/OpenSim/Region/Framework/Scenes/SceneViewer.cs b/OpenSim/Region/Framework/Scenes/SceneViewer.cs
index e2ea830..3e0d1db 100644
--- a/OpenSim/Region/Framework/Scenes/SceneViewer.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneViewer.cs
@@ -38,27 +38,42 @@ namespace OpenSim.Region.Framework.Scenes
 {
     public class SceneViewer : ISceneViewer
     {
+        /// <summary>
+        /// Is this scene viewer enabled?
+        /// </summary>
+        private bool IsEnabled { get; set; }
+
+        /// <summary>
+        /// The scene presence serviced by this viewer.
+        /// </summary>
         protected ScenePresence m_presence;
+
+        /// <summary>
+        /// The queue of parts for which we need to send out updates.
+        /// </summary>
         protected UpdateQueue m_partsUpdateQueue = new UpdateQueue();
+
+        /// <summary>
+        /// The queue of objects for which we need to send out updates.
+        /// </summary>
         protected Queue<SceneObjectGroup> m_pendingObjects;
 
+        /// <summary>
+        /// The last update assocated with a given part update.
+        /// </summary>
         protected Dictionary<UUID, ScenePartUpdate> m_updateTimes = new Dictionary<UUID, ScenePartUpdate>();
 
-        public SceneViewer()
-        {
-        }
-
         public SceneViewer(ScenePresence presence)
         {
             m_presence = presence;
+            IsEnabled = true;
         }
 
-        /// <summary>
-        /// Add the part to the queue of parts for which we need to send an update to the client
-        /// </summary>
-        /// <param name="part"></param>
         public void QueuePartForUpdate(SceneObjectPart part)
         {
+            if (!IsEnabled)
+                return;
+
             lock (m_partsUpdateQueue)
             {
                 m_partsUpdateQueue.Enqueue(part);
@@ -87,6 +102,11 @@ namespace OpenSim.Region.Framework.Scenes
 
             lock (m_pendingObjects)
             {
+                // We must do this under lock so that we don't suffer a race condition if another thread closes the
+                // viewer
+                if (!IsEnabled)
+                    return;
+
                 while (m_pendingObjects != null && m_pendingObjects.Count > 0)
                 {
                     SceneObjectGroup g = m_pendingObjects.Dequeue();
@@ -119,7 +139,6 @@ namespace OpenSim.Region.Framework.Scenes
 
                         // We deal with the possibility that two updates occur at
                         // the same unix time at the update point itself.
-
                         if ((update.LastFullUpdateTime < part.TimeStampFull) || part.ParentGroup.IsAttachment)
                         {
     //                            m_log.DebugFormat(
@@ -135,9 +154,7 @@ namespace OpenSim.Region.Framework.Scenes
                             // this update. If this happened, then subsequent
                             // updates which occurred on the same tick or the
                             // next tick of the last update would be ignored.
-
                             update.LastFullUpdateTime = part.TimeStampFull;
-
                         }
                         else if (update.LastTerseUpdateTime <= part.TimeStampTerse)
                         {
@@ -193,15 +210,21 @@ namespace OpenSim.Region.Framework.Scenes
 
         public void Close()
         {
-            lock (m_updateTimes)
-            {
-                m_updateTimes.Clear();
-            }
-            lock (m_partsUpdateQueue)
+            lock (m_pendingObjects)
             {
-                m_partsUpdateQueue.Clear();
+                IsEnabled = false;
+
+                lock (m_updateTimes)
+                {
+                    m_updateTimes.Clear();
+                }
+                lock (m_partsUpdateQueue)
+                {
+                    m_partsUpdateQueue.Clear();
+                }
+    
+                Reset();
             }
-            Reset();
         }
 
         public int GetPendingObjectsCount()
-- 
cgit v1.1