Resolve various race conditions between accessing and removing external script URLs by more consistently locking on m_UrlMap

1 files changed, 148 insertions, 132 deletions

diff --git a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
index 5c05500..05d54f0 100644
--- a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
+++ b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
@@ -41,13 +41,39 @@ using OpenSim.Region.Framework.Scenes;
 
 namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
 {
+    /// <summary>
+    /// Data describing an external URL set up by a script.
+    /// </summary>
     public class UrlData
     {
+        /// <summary>
+        /// Scene object part hosting the script
+        /// </summary>
         public UUID hostID;
+
+        /// <summary>
+        /// The item ID of the script that requested the URL.
+        /// </summary>
         public UUID itemID;
+
+        /// <summary>
+        /// The script engine that runs the script.
+        /// </summary>
         public IScriptModule engine;
+
+        /// <summary>
+        /// The generated URL.
+        /// </summary>
         public string url;
+
+        /// <summary>
+        /// The random UUID component of the generated URL.
+        /// </summary>
         public UUID urlcode;
+
+        /// <summary>
+        /// The external requests currently being processed or awaiting retrieval for this URL.
+        /// </summary>
         public Dictionary<UUID, RequestData> requests;
     }
 
@@ -77,6 +103,10 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
         /// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the request ID
         /// randomly generated when a request is received for this URL.
         /// </summary>
+        /// <remarks>
+        /// Manipulation or retrieval from this dictionary must be locked on m_UrlMap to preserve consistency with
+        /// m_UrlMap
+        /// </remarks>
         private Dictionary<UUID, UrlData> m_RequestMap = new Dictionary<UUID, UrlData>();
 
         /// <summary>
@@ -113,10 +143,9 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
         {
             m_ExternalHostNameForLSL = config.Configs["Network"].GetString("ExternalHostNameForLSL", System.Environment.MachineName);
             bool ssl_enabled = config.Configs["Network"].GetBoolean("https_listener",false);
+
             if (ssl_enabled)
-            {
                 https_port = (uint) config.Configs["Network"].GetInt("https_port",0);
-            }
 
             IConfig llFunctionsConfig = config.Configs["LL-Functions"];
 
@@ -275,32 +304,38 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
         
         public void HttpResponse(UUID request, int status, string body)
         {
-            if (m_RequestMap.ContainsKey(request))
-            {
-                UrlData urlData = m_RequestMap[request];
-                urlData.requests[request].responseCode = status;
-                urlData.requests[request].responseBody = body;
-                //urlData.requests[request].ev.Set();
-                urlData.requests[request].requestDone =true;
-            }
-            else
+            lock (m_UrlMap)
             {
-                m_log.Info("[HttpRequestHandler] There is no http-in request with id " + request.ToString());
+                if (m_RequestMap.ContainsKey(request))
+                {
+                    UrlData urlData = m_RequestMap[request];
+                    urlData.requests[request].responseCode = status;
+                    urlData.requests[request].responseBody = body;
+                    //urlData.requests[request].ev.Set();
+                    urlData.requests[request].requestDone =true;
+                }
+                else
+                {
+                    m_log.Info("[HttpRequestHandler] There is no http-in request with id " + request.ToString());
+                }
             }
         }
 
         public string GetHttpHeader(UUID requestId, string header)
         {
-            if (m_RequestMap.ContainsKey(requestId))
-            {
-                UrlData urlData = m_RequestMap[requestId];
-                string value;
-                if (urlData.requests[requestId].headers.TryGetValue(header,out value))
-                    return value;
-            }
-            else
+            lock (m_UrlMap)
             {
-                m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId);
+                if (m_RequestMap.ContainsKey(requestId))
+                {
+                    UrlData urlData = m_RequestMap[requestId];
+                    string value;
+                    if (urlData.requests[requestId].headers.TryGetValue(header, out value))
+                        return value;
+                }
+                else
+                {
+                    m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId);
+                }
             }
 
             return String.Empty;
@@ -308,7 +343,8 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
 
         public int GetFreeUrls()
         {
-            return m_TotalUrls - m_UrlMap.Count;
+            lock (m_UrlMap)
+                return m_TotalUrls - m_UrlMap.Count;
         }
 
         public void ScriptRemoved(UUID itemID)
@@ -366,9 +402,9 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
         private Hashtable NoEvents(UUID requestID, UUID sessionID)
         {
             Hashtable response = new Hashtable();
-            UrlData url;
+            UrlData urlData;
 
-            lock (m_RequestMap)
+            lock (m_UrlMap)
             {
                 // We need to return a 404 here in case the request URL was removed at exactly the same time that a
                 // request was made.  In this case, the request thread can outrace llRemoveURL() and still be polling
@@ -383,25 +419,22 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                     return response;
                 }
 
-                url = m_RequestMap[requestID];
-            }
-
-            if (System.Environment.TickCount - url.requests[requestID].startTime > 25000)
-            {
-                response["int_response_code"] = 500;
-                response["str_response_string"] = "Script timeout";
-                response["content_type"] = "text/plain";
-                response["keepalive"] = false;
-                response["reusecontext"] = false;
+                urlData = m_RequestMap[requestID];
 
-                //remove from map
-                lock (url)
+                if (System.Environment.TickCount - urlData.requests[requestID].startTime > 25000)
                 {
-                    url.requests.Remove(requestID);
+                    response["int_response_code"] = 500;
+                    response["str_response_string"] = "Script timeout";
+                    response["content_type"] = "text/plain";
+                    response["keepalive"] = false;
+                    response["reusecontext"] = false;
+    
+                    //remove from map
+                    urlData.requests.Remove(requestID);
                     m_RequestMap.Remove(requestID);
-                }
 
-                return response;
+                    return response;
+                }
             }
 
             return response;
@@ -409,9 +442,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
 
         private bool HasEvents(UUID requestID, UUID sessionID)
         {
-            UrlData url = null;
-            
-            lock (m_RequestMap)
+            lock (m_UrlMap)
             {
                 // We return true here because an external URL request that happened at the same time as an llRemoveURL()
                 // can still make it through to HttpRequestHandler().  That will return without setting up a request
@@ -423,61 +454,61 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                     return true;
                 }
 
-                url = m_RequestMap[requestID];
-                if (!url.requests.ContainsKey(requestID))
+                UrlData urlData = m_RequestMap[requestID];
+
+                if (!urlData.requests.ContainsKey(requestID))
                 {
                     return true;
                 }
-            }
 
-            // Trigger return of timeout response.
-            if (System.Environment.TickCount - url.requests[requestID].startTime > 25000)
-            {
-                return true;
-            }
+                // Trigger return of timeout response.
+                if (System.Environment.TickCount - urlData.requests[requestID].startTime > 25000)
+                {
+                    return true;
+                }
 
-            return url.requests[requestID].requestDone;
+                return urlData.requests[requestID].requestDone;
+            }
         }
 
         private Hashtable GetEvents(UUID requestID, UUID sessionID, string request)
         {
-            UrlData url = null;
-            RequestData requestData = null;
+            Hashtable response;
 
-            lock (m_RequestMap)
+            lock (m_UrlMap)
             {
+                UrlData url = null;
+                RequestData requestData = null;
+
                 if (!m_RequestMap.ContainsKey(requestID))
                     return NoEvents(requestID, sessionID);
 
                 url = m_RequestMap[requestID];
                 requestData = url.requests[requestID];
-            }
 
-            if (!requestData.requestDone)
-                return NoEvents(requestID, sessionID);
-            
-            Hashtable response = new Hashtable();
+                if (!requestData.requestDone)
+                    return NoEvents(requestID, sessionID);
 
-            if (System.Environment.TickCount - requestData.startTime > 25000)
-            {
-                response["int_response_code"] = 500;
-                response["str_response_string"] = "Script timeout";
+                response = new Hashtable();
+
+                if (System.Environment.TickCount - requestData.startTime > 25000)
+                {
+                    response["int_response_code"] = 500;
+                    response["str_response_string"] = "Script timeout";
+                    response["content_type"] = "text/plain";
+                    response["keepalive"] = false;
+                    response["reusecontext"] = false;
+                    return response;
+                }
+
+                //put response
+                response["int_response_code"] = requestData.responseCode;
+                response["str_response_string"] = requestData.responseBody;
                 response["content_type"] = "text/plain";
                 response["keepalive"] = false;
                 response["reusecontext"] = false;
-                return response;
-            }
 
-            //put response
-            response["int_response_code"] = requestData.responseCode;
-            response["str_response_string"] = requestData.responseBody;
-            response["content_type"] = "text/plain";
-            response["keepalive"] = false;
-            response["reusecontext"] = false;
-            
-            //remove from map
-            lock (url)
-            {
+                //remove from map
                 url.requests.Remove(requestID);
                 m_RequestMap.Remove(requestID);
             }
@@ -487,44 +518,41 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
 
         public void HttpRequestHandler(UUID requestID, Hashtable request)
         {
-            lock (request)
-            {
-                string uri = request["uri"].ToString();
-                bool is_ssl = uri.Contains("lslhttps");
+            string uri = request["uri"].ToString();
+            bool is_ssl = uri.Contains("lslhttps");
 
-                try
-                {
-                    Hashtable headers = (Hashtable)request["headers"];
+            try
+            {
+                Hashtable headers = (Hashtable)request["headers"];
 
 //                    string uri_full = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri;// "/lslhttp/" + urlcode.ToString() + "/";
 
-                    int pos1 = uri.IndexOf("/");// /lslhttp
-                    int pos2 = uri.IndexOf("/", pos1 + 1);// /lslhttp/
-                    int pos3 = uri.IndexOf("/", pos2 + 1);// /lslhttp/<UUID>/
-                    string uri_tmp = uri.Substring(0, pos3 + 1);
-                    //HTTP server code doesn't provide us with QueryStrings
-                    string pathInfo;
-                    string queryString;
-                    queryString = "";
+                int pos1 = uri.IndexOf("/");// /lslhttp
+                int pos2 = uri.IndexOf("/", pos1 + 1);// /lslhttp/
+                int pos3 = uri.IndexOf("/", pos2 + 1);// /lslhttp/<UUID>/
+                string uri_tmp = uri.Substring(0, pos3 + 1);
+                //HTTP server code doesn't provide us with QueryStrings
+                string pathInfo;
+                string queryString;
+                queryString = "";
 
-                    pathInfo = uri.Substring(pos3);
+                pathInfo = uri.Substring(pos3);
 
-                    UrlData urlData = null;
+                UrlData urlData = null;
 
-                    lock (m_UrlMap)
-                    {
-                        string url;
+                lock (m_UrlMap)
+                {
+                    string url;
 
-                        if (is_ssl)
-                            url = "https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp;
-                        else
-                            url = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp;
+                    if (is_ssl)
+                        url = "https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp;
+                    else
+                        url = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp;
 
-                        // Avoid a race - the request URL may have been released via llRequestUrl() whilst this
-                        // request was being processed.
-                        if (!m_UrlMap.TryGetValue(url, out urlData))
-                            return;
-                    }
+                    // Avoid a race - the request URL may have been released via llRequestUrl() whilst this
+                    // request was being processed.
+                    if (!m_UrlMap.TryGetValue(url, out urlData))
+                        return;
 
                     //for llGetHttpHeader support we need to store original URI here
                     //to make x-path-info / x-query-string / x-script-url / x-remote-ip headers 
@@ -544,6 +572,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                         string value = (string)header.Value;
                         requestData.headers.Add(key, value);
                     }
+
                     foreach (DictionaryEntry de in request)
                     {
                         if (de.Key.ToString() == "querystringkeys")
@@ -570,34 +599,21 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                     requestData.headers["x-query-string"] = queryString;
                     requestData.headers["x-script-url"] = urlData.url;
 
-                    //requestData.ev = new ManualResetEvent(false);
-                    lock (urlData.requests)
-                    {
-                        urlData.requests.Add(requestID, requestData);
-                    }
-
-                    lock (m_RequestMap)
-                    {
-                        m_RequestMap.Add(requestID, urlData);
-                    }
-
-                    urlData.engine.PostScriptEvent(
-                        urlData.itemID,
-                        "http_request",
-                        new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
-
-                    //send initial response?
-//                    Hashtable response = new Hashtable();
-
-                    return;
-                }
-                catch (Exception we)
-                {
-                    //Hashtable response = new Hashtable();
-                    m_log.Warn("[HttpRequestHandler]: http-in request failed");
-                    m_log.Warn(we.Message);
-                    m_log.Warn(we.StackTrace);
+                    urlData.requests.Add(requestID, requestData);
+                    m_RequestMap.Add(requestID, urlData);
                 }
+
+                urlData.engine.PostScriptEvent(
+                    urlData.itemID,
+                    "http_request",
+                    new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
+            }
+            catch (Exception we)
+            {
+                //Hashtable response = new Hashtable();
+                m_log.Warn("[HttpRequestHandler]: http-in request failed");
+                m_log.Warn(we.Message);
+                m_log.Warn(we.StackTrace);
             }
         }
 
@@ -606,4 +622,4 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
             ScriptRemoved(itemID);
         }
     }
-}
+}
\ No newline at end of file