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. --- .../Scripting/HttpRequest/ScriptsHttpRequests.cs | 92 +++++++++++++++------- .../Region/Framework/Interfaces/IHttpRequests.cs | 8 +- .../Api/Implementation/AsyncCommandManager.cs | 8 +- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs b/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs index c2e37c4..1c251b8 100644 --- a/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs +++ b/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs @@ -28,12 +28,15 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net; using System.Net.Mail; using System.Net.Security; +using System.Reflection; using System.Text; using System.Threading; using System.Security.Cryptography.X509Certificates; +using log4net; using Nini.Config; using OpenMetaverse; using OpenSim.Framework; @@ -250,18 +253,29 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest return reqID; } - public void StopHttpRequest(uint m_localID, UUID m_itemID) + public void StopHttpRequestsForScript(UUID id) { if (m_pendingRequests != null) { + List keysToRemove = null; + lock (HttpListLock) { - HttpRequestClass tmpReq; - if (m_pendingRequests.TryGetValue(m_itemID, out tmpReq)) + foreach (HttpRequestClass req in m_pendingRequests.Values) { - tmpReq.Stop(); - m_pendingRequests.Remove(m_itemID); + if (req.ItemID == id) + { + req.Stop(); + + if (keysToRemove == null) + keysToRemove = new List(); + + keysToRemove.Add(req.ReqID); + } } + + if (keysToRemove != null) + keysToRemove.ForEach(keyToRemove => m_pendingRequests.Remove(keyToRemove)); } } } @@ -362,6 +376,8 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest public class HttpRequestClass: IServiceRequest { +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); + // Constants for parameters // public const int HTTP_BODY_MAXLENGTH = 2; // public const int HTTP_METHOD = 0; @@ -419,12 +435,7 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest public void Process() { - httpThread = new Thread(SendRequest); - httpThread.Name = "HttpRequestThread"; - httpThread.Priority = ThreadPriority.BelowNormal; - httpThread.IsBackground = true; - _finished = false; - httpThread.Start(); + SendRequest(); } /* @@ -435,10 +446,6 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest public void SendRequest() { HttpWebResponse response = null; - StringBuilder sb = new StringBuilder(); - byte[] buf = new byte[8192]; - string tempString = null; - int count = 0; try { @@ -497,11 +504,12 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest bstream.Close(); } - Request.Timeout = HttpTimeout; try { - // execute the request - response = (HttpWebResponse) Request.GetResponse(); + IAsyncResult result = (IAsyncResult)Request.BeginGetResponse(ResponseCallback, null); + + ThreadPool.RegisterWaitForSingleObject( + result.AsyncWaitHandle, new WaitOrTimerCallback(TimeoutCallback), null, HttpTimeout, true); } catch (WebException e) { @@ -510,11 +518,31 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest throw; } response = (HttpWebResponse)e.Response; + _finished = true; } + } + catch (Exception e) + { + Status = (int)OSHttpStatusCode.ClientErrorJoker; + ResponseBody = e.Message; + _finished = true; + } + } + private void ResponseCallback(IAsyncResult ar) + { + HttpWebResponse response = null; + + try + { + response = (HttpWebResponse)Request.EndGetResponse(ar); Status = (int)response.StatusCode; Stream resStream = response.GetResponseStream(); + StringBuilder sb = new StringBuilder(); + byte[] buf = new byte[8192]; + string tempString = null; + int count = 0; do { @@ -530,36 +558,40 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest // continue building the string sb.Append(tempString); } - } while (count > 0); // any more data to read? + } + while (count > 0); // any more data to read? - ResponseBody = sb.ToString(); + ResponseBody = sb.ToString(); } catch (Exception e) { Status = (int)OSHttpStatusCode.ClientErrorJoker; ResponseBody = e.Message; - _finished = true; - return; +// m_log.Debug( +// string.Format("[SCRIPTS HTTP REQUESTS]: Exception on response to {0} for {1} ", Url, ItemID), e); } finally { if (response != null) response.Close(); + + _finished = true; } + } - _finished = true; + private void TimeoutCallback(object state, bool timedOut) + { + if (timedOut) + Request.Abort(); } public void Stop() { - try - { - httpThread.Abort(); - } - catch (Exception) - { - } +// m_log.DebugFormat("[SCRIPTS HTTP REQUESTS]: Stopping request to {0} for {1} ", Url, ItemID); + + if (Request != null) + Request.Abort(); } } } diff --git a/OpenSim/Region/Framework/Interfaces/IHttpRequests.cs b/OpenSim/Region/Framework/Interfaces/IHttpRequests.cs index eb6c5ac..113dcd7 100644 --- a/OpenSim/Region/Framework/Interfaces/IHttpRequests.cs +++ b/OpenSim/Region/Framework/Interfaces/IHttpRequests.cs @@ -45,7 +45,13 @@ namespace OpenSim.Region.Framework.Interfaces { UUID MakeHttpRequest(string url, string parameters, string body); UUID StartHttpRequest(uint localID, UUID itemID, string url, List parameters, Dictionary headers, string body); - void StopHttpRequest(uint m_localID, UUID m_itemID); + + /// + /// Stop and remove all http requests for the given script. + /// + /// + void StopHttpRequestsForScript(UUID id); + IServiceRequest GetNextCompletedRequest(); void RemoveCompletedRequest(UUID id); } 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 f281a994e8544d95961ef669a0fe14c0cba7f175 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 4 Apr 2013 00:49:07 +0100 Subject: refactor: Simplify ScriptsHttpRequests.GetNextCompletedRequest to more simply iterate through pending requests without unnecessary checks. --- .../Scripting/HttpRequest/ScriptsHttpRequests.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs b/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs index 1c251b8..ebf56cd 100644 --- a/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs +++ b/OpenSim/Region/CoreModules/Scripting/HttpRequest/ScriptsHttpRequests.cs @@ -293,19 +293,13 @@ namespace OpenSim.Region.CoreModules.Scripting.HttpRequest { lock (HttpListLock) { - foreach (UUID luid in m_pendingRequests.Keys) + foreach (HttpRequestClass req in m_pendingRequests.Values) { - HttpRequestClass tmpReq; - - if (m_pendingRequests.TryGetValue(luid, out tmpReq)) - { - if (tmpReq.Finished) - { - return tmpReq; - } - } + if (req.Finished) + return req; } } + return null; } -- cgit v1.1