Lock areas of AsyncCommandManager where multiple threads could try to access/update the same static structures simultaneously.
This is possible where there is more than one scene (multiple copies of the same script engine) and/or more than one script engine being used. These operations are not thread safe and could be leading to the exceptions/problems seen in http://opensimulator.org/mantis/view.php?id=6651 This also prevents a small race condition where more than one AsyncLSLCmdHandlerThread could be started.cpu-performance
							parent
							
								
									00c1586ff8
								
							
						
					
					
						commit
						921ad8704e
					
				|  | @ -52,6 +52,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         private static Thread cmdHandlerThread; | ||||
|         private static int cmdHandlerThreadCycleSleepms; | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Lock for reading/writing static components of AsyncCommandManager. | ||||
|         /// </summary> | ||||
|         /// <remarks> | ||||
|         /// This lock exists so that multiple threads from different engines and/or different copies of the same engine | ||||
|         /// are prevented from running non-thread safe code (e.g. read/write of lists) concurrently. | ||||
|         /// </remarks> | ||||
|         private static object staticLock = new object(); | ||||
| 
 | ||||
|         private static List<IScene> m_Scenes = new List<IScene>(); | ||||
|         private static List<IScriptEngine> m_ScriptEngines = | ||||
|                 new List<IScriptEngine>(); | ||||
|  | @ -74,37 +83,65 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
| 
 | ||||
|         public Dataserver DataserverPlugin | ||||
|         { | ||||
|             get { return m_Dataserver[m_ScriptEngine]; } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_Dataserver[m_ScriptEngine];  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public Timer TimerPlugin | ||||
|         { | ||||
|             get { return m_Timer[m_ScriptEngine]; } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_Timer[m_ScriptEngine];  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public HttpRequest HttpRequestPlugin | ||||
|         { | ||||
|             get { return m_HttpRequest[m_ScriptEngine]; } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_HttpRequest[m_ScriptEngine];  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public Listener ListenerPlugin | ||||
|         { | ||||
|             get { return m_Listener[m_ScriptEngine]; } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_Listener[m_ScriptEngine];  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public SensorRepeat SensorRepeatPlugin | ||||
|         { | ||||
|             get { return m_SensorRepeat[m_ScriptEngine]; } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_SensorRepeat[m_ScriptEngine];  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public XmlRequest XmlRequestPlugin | ||||
|         { | ||||
|             get { return m_XmlRequest[m_ScriptEngine]; } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_XmlRequest[m_ScriptEngine];  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public IScriptEngine[] ScriptEngines | ||||
|         { | ||||
|             get { return m_ScriptEngines.ToArray(); } | ||||
|             get  | ||||
|             {  | ||||
|                 lock (staticLock) | ||||
|                     return m_ScriptEngines.ToArray();  | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public AsyncCommandManager(IScriptEngine _ScriptEngine) | ||||
|  | @ -112,6 +149,12 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|             m_ScriptEngine = _ScriptEngine; | ||||
|             m_Scene = m_ScriptEngine.World; | ||||
| 
 | ||||
|             // If there is more than one scene in the simulator or multiple script engines are used on the same region | ||||
|             // then more than one thread could arrive at this block of code simultaneously.  However, it cannot be | ||||
|             // executed concurrently both because concurrent list operations are not thread-safe and because of other | ||||
|             // race conditions such as the later check of cmdHandlerThread == null. | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 if (m_Scenes.Count == 0) | ||||
|                     ReadConfig(); | ||||
| 
 | ||||
|  | @ -136,6 +179,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
| 
 | ||||
|                 StartThread(); | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         private static void StartThread() | ||||
|         { | ||||
|  | @ -197,6 +241,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         } | ||||
| 
 | ||||
|         private static void DoOneCmdHandlerPass() | ||||
|         { | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 // Check HttpRequests | ||||
|                 m_HttpRequest[m_ScriptEngines[0]].CheckHttpRequests(); | ||||
|  | @ -219,6 +265,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|                     m_Dataserver[s].ExpireRequests(); | ||||
|                 } | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Remove a specific script (and all its pending commands) | ||||
|  | @ -229,8 +276,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         { | ||||
| //            m_log.DebugFormat("[ASYNC COMMAND MANAGER]: Removing facilities for script {0}", itemID); | ||||
| 
 | ||||
|             // Remove a specific script | ||||
| 
 | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 // Remove dataserver events | ||||
|                 m_Dataserver[engine].RemoveEvents(localID, itemID); | ||||
| 
 | ||||
|  | @ -256,6 +303,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|                 // Remove Sensors | ||||
|                 m_SensorRepeat[engine].UnSetSenseRepeaterEvents(localID, itemID); | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Get the sensor repeat plugin for this script engine. | ||||
|  | @ -263,12 +311,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         /// <param name="engine"></param> | ||||
|         /// <returns></returns> | ||||
|         public static SensorRepeat GetSensorRepeatPlugin(IScriptEngine engine) | ||||
|         { | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 if (m_SensorRepeat.ContainsKey(engine)) | ||||
|                     return m_SensorRepeat[engine]; | ||||
|                 else | ||||
|                     return null; | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Get the dataserver plugin for this script engine. | ||||
|  | @ -276,12 +327,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         /// <param name="engine"></param> | ||||
|         /// <returns></returns> | ||||
|         public static Dataserver GetDataserverPlugin(IScriptEngine engine) | ||||
|         { | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 if (m_Dataserver.ContainsKey(engine)) | ||||
|                     return m_Dataserver[engine]; | ||||
|                 else | ||||
|                     return null; | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Get the timer plugin for this script engine. | ||||
|  | @ -289,12 +343,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         /// <param name="engine"></param> | ||||
|         /// <returns></returns> | ||||
|         public static Timer GetTimerPlugin(IScriptEngine engine) | ||||
|         { | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 if (m_Timer.ContainsKey(engine)) | ||||
|                     return m_Timer[engine]; | ||||
|                 else | ||||
|                     return null; | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Get the listener plugin for this script engine. | ||||
|  | @ -302,17 +359,22 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|         /// <param name="engine"></param> | ||||
|         /// <returns></returns> | ||||
|         public static Listener GetListenerPlugin(IScriptEngine engine) | ||||
|         { | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 if (m_Listener.ContainsKey(engine)) | ||||
|                     return m_Listener[engine]; | ||||
|                 else | ||||
|                     return null; | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public static Object[] GetSerializationData(IScriptEngine engine, UUID itemID) | ||||
|         { | ||||
|             List<Object> data = new List<Object>(); | ||||
| 
 | ||||
|             lock (staticLock) | ||||
|             { | ||||
|                 Object[] listeners = m_Listener[engine].GetSerializationData(itemID); | ||||
|                 if (listeners.Length > 0) | ||||
|                 { | ||||
|  | @ -336,6 +398,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|                     data.Add(sensors.Length); | ||||
|                     data.AddRange(sensors); | ||||
|                 } | ||||
|             } | ||||
| 
 | ||||
|             return data.ToArray(); | ||||
|         } | ||||
|  | @ -359,6 +422,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
| 
 | ||||
|                     idx+=len; | ||||
| 
 | ||||
|                     lock (staticLock) | ||||
|                     { | ||||
|                     switch (type) | ||||
|                     { | ||||
|                         case "listener": | ||||
|  | @ -379,3 +444,4 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api | |||
|             } | ||||
|         } | ||||
|     } | ||||
| } | ||||
		Loading…
	
		Reference in New Issue
	
	 Justin Clark-Casey (justincc)
						Justin Clark-Casey (justincc)