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.
0.7.3-extended
Justin Clark-Casey (justincc) 2012-07-31 23:57:57 +01:00
parent ec063b9088
commit 1d0ff7da2a
1 changed files with 43 additions and 35 deletions

View File

@ -51,7 +51,6 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
{ {
get get
{ {
lock (SenseRepeatListLock)
return SenseRepeaters.Count; return SenseRepeaters.Count;
} }
} }
@ -115,6 +114,16 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
public double distance; 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 List<SenseRepeatClass> SenseRepeaters = new List<SenseRepeatClass>();
private object SenseRepeatListLock = new object(); private object SenseRepeatListLock = new object();
@ -124,6 +133,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
{ {
// Always remove first, in case this is a re-set // Always remove first, in case this is a re-set
UnSetSenseRepeaterEvents(m_localID, m_itemID); UnSetSenseRepeaterEvents(m_localID, m_itemID);
if (sec == 0) // Disabling timer if (sec == 0) // Disabling timer
return; return;
@ -143,9 +153,17 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
ts.host = host; ts.host = host;
ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval); ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
AddSenseRepeater(ts);
}
private void AddSenseRepeater(SenseRepeatClass senseRepeater)
{
lock (SenseRepeatListLock) lock (SenseRepeatListLock)
{ {
SenseRepeaters.Add(ts); List<SenseRepeatClass> newSenseRepeaters = new List<SenseRepeatClass>(SenseRepeaters);
newSenseRepeaters.Add(senseRepeater);
SenseRepeaters = newSenseRepeaters;
} }
} }
@ -154,27 +172,21 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
// Remove from timer // Remove from timer
lock (SenseRepeatListLock) lock (SenseRepeatListLock)
{ {
List<SenseRepeatClass> NewSensors = new List<SenseRepeatClass>(); List<SenseRepeatClass> newSenseRepeaters = new List<SenseRepeatClass>();
foreach (SenseRepeatClass ts in SenseRepeaters) foreach (SenseRepeatClass ts in SenseRepeaters)
{ {
if (ts.localID != m_localID || ts.itemID != m_itemID) if (ts.localID != m_localID || ts.itemID != m_itemID)
{ {
NewSensors.Add(ts); newSenseRepeaters.Add(ts);
} }
} }
SenseRepeaters.Clear();
SenseRepeaters = NewSensors; SenseRepeaters = newSenseRepeaters;
} }
} }
public void CheckSenseRepeaterEvents() public void CheckSenseRepeaterEvents()
{ {
lock (SenseRepeatListLock)
{
// Nothing to do here?
if (SenseRepeaters.Count == 0)
return;
// Go through all timers // Go through all timers
foreach (SenseRepeatClass ts in SenseRepeaters) foreach (SenseRepeatClass ts in SenseRepeaters)
{ {
@ -186,7 +198,6 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval); ts.next = DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
} }
} }
} // lock
} }
public void SenseOnce(uint m_localID, UUID m_itemID, public void SenseOnce(uint m_localID, UUID m_itemID,
@ -599,8 +610,6 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
{ {
List<Object> data = new List<Object>(); 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)
@ -613,7 +622,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
data.Add(ts.arc); data.Add(ts.arc);
} }
} }
}
return data.ToArray(); return data.ToArray();
} }
@ -647,8 +656,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api.Plugins
ts.next = ts.next =
DateTime.Now.ToUniversalTime().AddSeconds(ts.interval); DateTime.Now.ToUniversalTime().AddSeconds(ts.interval);
lock (SenseRepeatListLock) AddSenseRepeater(ts);
SenseRepeaters.Add(ts);
idx += 6; idx += 6;
} }