From 831e4c38506140e9ece2db4b96b4f0960a0276a8 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 4 Apr 2013 00:36:15 +0100 Subject: Fix bug where outstanding llHTTPRequests for scripts were not being aborted when they were deleted. This was because AsyncCommandManager was handing an item ID to IHttpRequestModule.StopHttpRequest() rather than the expected request ID. This commit also makes the http request asynchronous using BeginGetResponse() rather than doing this by launching a new thread so that we can more safely abort it via HttpWebRequest.Abort() rather than aborting the thread itself. This also renames StopHttpRequest() to StopHttpRequestsForScript() since any outstanding requests are now aborted and/or removed. --- .../ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs') diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs index 47a9cdc..1c59624 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs @@ -28,7 +28,9 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Reflection; using System.Threading; +using log4net; using OpenMetaverse; using OpenSim.Framework; using OpenSim.Framework.Monitoring; @@ -45,6 +47,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public class AsyncCommandManager { +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); + private static Thread cmdHandlerThread; private static int cmdHandlerThreadCycleSleepms; @@ -225,6 +229,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public static void RemoveScript(IScriptEngine engine, uint localID, UUID itemID) { +// m_log.DebugFormat("[ASYNC COMMAND MANAGER]: Removing facilities for script {0}", itemID); + // Remove a specific script // Remove dataserver events @@ -236,7 +242,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api // Remove from: HttpRequest IHttpRequestModule iHttpReq = engine.World.RequestModuleInterface(); if (iHttpReq != null) - iHttpReq.StopHttpRequest(localID, itemID); + iHttpReq.StopHttpRequestsForScript(itemID); IWorldComm comms = engine.World.RequestModuleInterface(); if (comms != null) -- cgit v1.1 From 46335b103e1f993c6aa77e46f89e5ee0d6d7497e Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 30 May 2013 23:51:35 +0100 Subject: If an exception occurs in the AsyncCommandManager loop, spit it out to log rather than silently swallowing it. This might help diagnose the cause of http://opensimulator.org/mantis/view.php?id=6651 where sometimes scripts fail to start on region start. --- .../Shared/Api/Implementation/AsyncCommandManager.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs') diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs index 1c59624..352e316 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs @@ -47,7 +47,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public class AsyncCommandManager { -// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); + private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); private static Thread cmdHandlerThread; private static int cmdHandlerThreadCycleSleepms; @@ -183,17 +183,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api { try { - while (true) - { - Thread.Sleep(cmdHandlerThreadCycleSleepms); + Thread.Sleep(cmdHandlerThreadCycleSleepms); - DoOneCmdHandlerPass(); + DoOneCmdHandlerPass(); - Watchdog.UpdateThread(); - } + Watchdog.UpdateThread(); } - catch + catch (Exception e) { + m_log.Error("[ASYNC COMMAND MANAGER]: Exception in command handler pass: ", e); } } } -- cgit v1.1 From 00c1586ff8cd0abb2270a109087857adefe3b5c2 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 31 May 2013 18:12:36 +0100 Subject: refactor: Remove unused AsyncCommandManager.PleaseShutdown --- .../Shared/Api/Implementation/AsyncCommandManager.cs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) (limited to 'OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs') diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs index 352e316..c71b571 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs @@ -377,23 +377,5 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api } } } - - #region Check llRemoteData channels - - #endregion - - #region Check llListeners - - #endregion - - /// - /// If set to true then threads and stuff should try to make a graceful exit - /// - public bool PleaseShutdown - { - get { return _PleaseShutdown; } - set { _PleaseShutdown = value; } - } - private bool _PleaseShutdown = false; } -} +} \ No newline at end of file -- cgit v1.1 From 921ad8704e08fccc994f7907e7cb1e9372e355b9 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 31 May 2013 22:50:15 +0100 Subject: 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. --- .../Api/Implementation/AsyncCommandManager.cs | 288 +++++++++++++-------- 1 file changed, 177 insertions(+), 111 deletions(-) (limited to 'OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs') diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs index c71b571..72e8415 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs @@ -52,6 +52,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api private static Thread cmdHandlerThread; private static int cmdHandlerThreadCycleSleepms; + /// + /// Lock for reading/writing static components of AsyncCommandManager. + /// + /// + /// 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. + /// + private static object staticLock = new object(); + private static List m_Scenes = new List(); private static List m_ScriptEngines = new List(); @@ -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,29 +149,36 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api m_ScriptEngine = _ScriptEngine; m_Scene = m_ScriptEngine.World; - if (m_Scenes.Count == 0) - ReadConfig(); - - if (!m_Scenes.Contains(m_Scene)) - m_Scenes.Add(m_Scene); - if (!m_ScriptEngines.Contains(m_ScriptEngine)) - m_ScriptEngines.Add(m_ScriptEngine); - - // Create instances of all plugins - if (!m_Dataserver.ContainsKey(m_ScriptEngine)) - m_Dataserver[m_ScriptEngine] = new Dataserver(this); - if (!m_Timer.ContainsKey(m_ScriptEngine)) - m_Timer[m_ScriptEngine] = new Timer(this); - if (!m_HttpRequest.ContainsKey(m_ScriptEngine)) - m_HttpRequest[m_ScriptEngine] = new HttpRequest(this); - if (!m_Listener.ContainsKey(m_ScriptEngine)) - m_Listener[m_ScriptEngine] = new Listener(this); - if (!m_SensorRepeat.ContainsKey(m_ScriptEngine)) - m_SensorRepeat[m_ScriptEngine] = new SensorRepeat(this); - if (!m_XmlRequest.ContainsKey(m_ScriptEngine)) - m_XmlRequest[m_ScriptEngine] = new XmlRequest(this); - - StartThread(); + // 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(); + + if (!m_Scenes.Contains(m_Scene)) + m_Scenes.Add(m_Scene); + if (!m_ScriptEngines.Contains(m_ScriptEngine)) + m_ScriptEngines.Add(m_ScriptEngine); + + // Create instances of all plugins + if (!m_Dataserver.ContainsKey(m_ScriptEngine)) + m_Dataserver[m_ScriptEngine] = new Dataserver(this); + if (!m_Timer.ContainsKey(m_ScriptEngine)) + m_Timer[m_ScriptEngine] = new Timer(this); + if (!m_HttpRequest.ContainsKey(m_ScriptEngine)) + m_HttpRequest[m_ScriptEngine] = new HttpRequest(this); + if (!m_Listener.ContainsKey(m_ScriptEngine)) + m_Listener[m_ScriptEngine] = new Listener(this); + if (!m_SensorRepeat.ContainsKey(m_ScriptEngine)) + m_SensorRepeat[m_ScriptEngine] = new SensorRepeat(this); + if (!m_XmlRequest.ContainsKey(m_ScriptEngine)) + m_XmlRequest[m_ScriptEngine] = new XmlRequest(this); + + StartThread(); + } } private static void StartThread() @@ -198,25 +242,28 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api private static void DoOneCmdHandlerPass() { - // Check HttpRequests - m_HttpRequest[m_ScriptEngines[0]].CheckHttpRequests(); + lock (staticLock) + { + // Check HttpRequests + m_HttpRequest[m_ScriptEngines[0]].CheckHttpRequests(); - // Check XMLRPCRequests - m_XmlRequest[m_ScriptEngines[0]].CheckXMLRPCRequests(); + // Check XMLRPCRequests + m_XmlRequest[m_ScriptEngines[0]].CheckXMLRPCRequests(); - foreach (IScriptEngine s in m_ScriptEngines) - { - // Check Listeners - m_Listener[s].CheckListeners(); + foreach (IScriptEngine s in m_ScriptEngines) + { + // Check Listeners + m_Listener[s].CheckListeners(); - // Check timers - m_Timer[s].CheckTimerEvents(); + // Check timers + m_Timer[s].CheckTimerEvents(); - // Check Sensors - m_SensorRepeat[s].CheckSenseRepeaterEvents(); + // Check Sensors + m_SensorRepeat[s].CheckSenseRepeaterEvents(); - // Check dataserver - m_Dataserver[s].ExpireRequests(); + // Check dataserver + m_Dataserver[s].ExpireRequests(); + } } } @@ -229,32 +276,33 @@ 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); - // Remove dataserver events - m_Dataserver[engine].RemoveEvents(localID, itemID); + // Remove from: Timers + m_Timer[engine].UnSetTimerEvents(localID, itemID); - // Remove from: Timers - m_Timer[engine].UnSetTimerEvents(localID, itemID); + // Remove from: HttpRequest + IHttpRequestModule iHttpReq = engine.World.RequestModuleInterface(); + if (iHttpReq != null) + iHttpReq.StopHttpRequestsForScript(itemID); - // Remove from: HttpRequest - IHttpRequestModule iHttpReq = engine.World.RequestModuleInterface(); - if (iHttpReq != null) - iHttpReq.StopHttpRequestsForScript(itemID); + IWorldComm comms = engine.World.RequestModuleInterface(); + if (comms != null) + comms.DeleteListener(itemID); - IWorldComm comms = engine.World.RequestModuleInterface(); - if (comms != null) - comms.DeleteListener(itemID); + IXMLRPC xmlrpc = engine.World.RequestModuleInterface(); + if (xmlrpc != null) + { + xmlrpc.DeleteChannels(itemID); + xmlrpc.CancelSRDRequests(itemID); + } - IXMLRPC xmlrpc = engine.World.RequestModuleInterface(); - if (xmlrpc != null) - { - xmlrpc.DeleteChannels(itemID); - xmlrpc.CancelSRDRequests(itemID); + // Remove Sensors + m_SensorRepeat[engine].UnSetSenseRepeaterEvents(localID, itemID); } - - // Remove Sensors - m_SensorRepeat[engine].UnSetSenseRepeaterEvents(localID, itemID); } /// @@ -264,10 +312,13 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public static SensorRepeat GetSensorRepeatPlugin(IScriptEngine engine) { - if (m_SensorRepeat.ContainsKey(engine)) - return m_SensorRepeat[engine]; - else - return null; + lock (staticLock) + { + if (m_SensorRepeat.ContainsKey(engine)) + return m_SensorRepeat[engine]; + else + return null; + } } /// @@ -277,10 +328,13 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public static Dataserver GetDataserverPlugin(IScriptEngine engine) { - if (m_Dataserver.ContainsKey(engine)) - return m_Dataserver[engine]; - else - return null; + lock (staticLock) + { + if (m_Dataserver.ContainsKey(engine)) + return m_Dataserver[engine]; + else + return null; + } } /// @@ -290,10 +344,13 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public static Timer GetTimerPlugin(IScriptEngine engine) { - if (m_Timer.ContainsKey(engine)) - return m_Timer[engine]; - else - return null; + lock (staticLock) + { + if (m_Timer.ContainsKey(engine)) + return m_Timer[engine]; + else + return null; + } } /// @@ -303,38 +360,44 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// public static Listener GetListenerPlugin(IScriptEngine engine) { - if (m_Listener.ContainsKey(engine)) - return m_Listener[engine]; - else - return null; + lock (staticLock) + { + if (m_Listener.ContainsKey(engine)) + return m_Listener[engine]; + else + return null; + } } public static Object[] GetSerializationData(IScriptEngine engine, UUID itemID) { List data = new List(); - Object[] listeners = m_Listener[engine].GetSerializationData(itemID); - if (listeners.Length > 0) + lock (staticLock) { - data.Add("listener"); - data.Add(listeners.Length); - data.AddRange(listeners); - } + Object[] listeners = m_Listener[engine].GetSerializationData(itemID); + if (listeners.Length > 0) + { + data.Add("listener"); + data.Add(listeners.Length); + data.AddRange(listeners); + } - Object[] timers=m_Timer[engine].GetSerializationData(itemID); - if (timers.Length > 0) - { - data.Add("timer"); - data.Add(timers.Length); - data.AddRange(timers); - } + Object[] timers=m_Timer[engine].GetSerializationData(itemID); + if (timers.Length > 0) + { + data.Add("timer"); + data.Add(timers.Length); + data.AddRange(timers); + } - Object[] sensors = m_SensorRepeat[engine].GetSerializationData(itemID); - if (sensors.Length > 0) - { - data.Add("sensor"); - data.Add(sensors.Length); - data.AddRange(sensors); + Object[] sensors = m_SensorRepeat[engine].GetSerializationData(itemID); + if (sensors.Length > 0) + { + data.Add("sensor"); + data.Add(sensors.Length); + data.AddRange(sensors); + } } return data.ToArray(); @@ -359,20 +422,23 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api idx+=len; + lock (staticLock) + { switch (type) { - case "listener": - m_Listener[engine].CreateFromData(localID, itemID, - hostID, item); - break; - case "timer": - m_Timer[engine].CreateFromData(localID, itemID, - hostID, item); - break; - case "sensor": - m_SensorRepeat[engine].CreateFromData(localID, - itemID, hostID, item); - break; + case "listener": + m_Listener[engine].CreateFromData(localID, itemID, + hostID, item); + break; + case "timer": + m_Timer[engine].CreateFromData(localID, itemID, + hostID, item); + break; + case "sensor": + m_SensorRepeat[engine].CreateFromData(localID, + itemID, hostID, item); + break; + } } } } -- cgit v1.1 From 217c7d11401dbf15ada3de750e7ad0df85a02894 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 31 May 2013 23:00:10 +0100 Subject: Remove unnecessary m_scenes and m_scene from AsyncCommandManager. These were private and the sole point of use (to know when to load config for the first time) can be done by looking at script engines instead. --- .../ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs') diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs index 72e8415..998f40b 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/AsyncCommandManager.cs @@ -61,12 +61,10 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api /// private static object staticLock = new object(); - private static List m_Scenes = new List(); private static List m_ScriptEngines = new List(); public IScriptEngine m_ScriptEngine; - private IScene m_Scene; private static Dictionary m_Dataserver = new Dictionary(); @@ -147,7 +145,6 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api public AsyncCommandManager(IScriptEngine _ScriptEngine) { 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 @@ -155,11 +152,9 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api // race conditions such as the later check of cmdHandlerThread == null. lock (staticLock) { - if (m_Scenes.Count == 0) + if (m_ScriptEngines.Count == 0) ReadConfig(); - if (!m_Scenes.Contains(m_Scene)) - m_Scenes.Add(m_Scene); if (!m_ScriptEngines.Contains(m_ScriptEngine)) m_ScriptEngines.Add(m_ScriptEngine); -- cgit v1.1