Resolve a deadlock between INPCModule and SensorRepeat by replacing the SensorRepeat list with a new list on add/removes rather than locking it for the duration of the sensor sweep.

A deadlock was observed today where NPC removal on a script thread would lock the NPC list and then try to lock the sensor list via scripted attachment removal.
Concurrently, the sensor sweep thread would lock the sensor list and then try to lock the NPC list to check NPC status.
This commit resolves the deadlock by replacing the sensor list on update rather than locking it for the duration of the sweep.

1 files changed, 43 insertions, 35 deletions

diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/Plugins/SensorRepeat.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/Plugins/SensorRepeat.cs
index f2c8b60..06495bb 100644
--- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/Plugins/SensorRepeat.cs
+++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/Plugins/SensorRepeat.cs
@@ -51,8 +51,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
         {
             get
             {
-                lock (SenseRepeatListLock)
-                    return SenseRepeaters.Count;
+                return SenseRepeaters.Count;
             }
         }
 
@@ -116,6 +115,16 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
             public double distance;
         }
 
+        /// <summary>
+        /// Sensors to process.
+        /// </summary>
+        /// <remarks>
+        /// Do not add or remove sensors from this list directly.  Instead, copy the list and substitute the updated
+        /// copy.  This is to avoid locking the list for the duration of the sensor sweep, which increases the danger
+        /// of deadlocks with future code updates.
+        ///
+        /// Always lock SenseRepeatListLock when updating this list.
+        /// </remarks>
         private List<SenseRepeatClass> SenseRepeaters = new List<SenseRepeatClass>();
         private object SenseRepeatListLock = new object();
 
@@ -125,6 +134,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
         {
             // Always remove first, in case this is a re-set
             UnSetSenseRepeaterEvents(m_localID, m_itemID);
+
             if (sec == 0) // Disabling timer
                 return;
 
@@ -144,9 +154,17 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
             ts.host = host;
 
             ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
+
+            AddSenseRepeater(ts);
+        }
+
+        private void AddSenseRepeater(SenseRepeatClass senseRepeater)
+        {
             lock (SenseRepeatListLock)
             {
-                SenseRepeaters.Add(ts);
+                List<SenseRepeatClass> newSenseRepeaters = new List<SenseRepeatClass>(SenseRepeaters);
+                newSenseRepeaters.Add(senseRepeater);
+                SenseRepeaters = newSenseRepeaters;
             }
         }
 
@@ -155,39 +173,32 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
             // Remove from timer
             lock (SenseRepeatListLock)
             {
-                List<SenseRepeatClass> NewSensors = new List<SenseRepeatClass>();
+                List<SenseRepeatClass> newSenseRepeaters = new List<SenseRepeatClass>();
                 foreach (SenseRepeatClass ts in SenseRepeaters)
                 {
                     if (ts.localID != m_localID || ts.itemID != m_itemID)
                     {
-                        NewSensors.Add(ts);
+                        newSenseRepeaters.Add(ts);
                     }
                 }
-                SenseRepeaters.Clear();
-                SenseRepeaters = NewSensors;
+
+                SenseRepeaters = newSenseRepeaters;
             }
         }
 
         public void CheckSenseRepeaterEvents()
         {
-            lock (SenseRepeatListLock)
+            // Go through all timers
+            foreach (SenseRepeatClass ts in SenseRepeaters)
             {
-                // Nothing to do here?
-                if (SenseRepeaters.Count == 0)
-                    return;
-
-                // Go through all timers
-                foreach (SenseRepeatClass ts in SenseRepeaters)
+                // Time has passed?
+                if (ts.next.ToUniversalTime() < DateTime.Now.ToUniversalTime())
                 {
-                    // Time has passed?
-                    if (ts.next.ToUniversalTime() < DateTime.Now.ToUniversalTime())
-                    {
-                        SensorSweep(ts);
-                        // set next interval
-                        ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
-                    }
+                    SensorSweep(ts);
+                    // set next interval
+                    ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
                 }
-            } // lock
+            }
         }
 
         public void SenseOnce(uint m_localID, UUID m_itemID,
@@ -615,21 +626,19 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
         {
             List<Object> data = new List<Object>();
 
-            lock (SenseRepeatListLock)
+            foreach (SenseRepeatClass ts in SenseRepeaters)
             {
-                foreach (SenseRepeatClass ts in SenseRepeaters)
+                if (ts.itemID == itemID)
                 {
-                    if (ts.itemID == itemID)
-                    {
-                        data.Add(ts.interval);
-                        data.Add(ts.name);
-                        data.Add(ts.keyID);
-                        data.Add(ts.type);
-                        data.Add(ts.range);
-                        data.Add(ts.arc);
-                    }
+                    data.Add(ts.interval);
+                    data.Add(ts.name);
+                    data.Add(ts.keyID);
+                    data.Add(ts.type);
+                    data.Add(ts.range);
+                    data.Add(ts.arc);
                 }
             }
+
             return data.ToArray();
         }
 
@@ -663,8 +672,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
                 ts.next =
                     DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
 
-                lock (SenseRepeatListLock)
-                    SenseRepeaters.Add(ts);
+                AddSenseRepeater(ts);
                 
                 idx += 6;
             }