Avoid a race condition where an incoming request to a script external URL can trigger an exception is the URL was being removed at the same time.

This involves three steps
1) Return gracefully in UrlModule.HttpRequestHandler() instead of throwing an exception when the url cannot be found in its index
2) Return true instead of false in HasEvents() if no matching request is found in the map.  This call will only happen in the first place for raced requests.
3) Return a 404 in GetEvents() if the request is not in the index, rather than a blank 200 OK.

Many thanks to Tom Haines in http://opensimulator.org/mantis/view.php?id=6051 for doing some of the work on this.

3 files changed, 84 insertions, 40 deletions

diff --git a/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs b/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs
index 9d512c6..3089351 100644
--- a/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs
+++ b/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs
@@ -28,6 +28,7 @@
 using System;
 using System.Collections;
 using OpenMetaverse;
+
 namespace OpenSim.Framework.Servers.HttpServer
 {
     public delegate void RequestMethod(UUID requestID, Hashtable request);
@@ -44,7 +45,11 @@ namespace OpenSim.Framework.Servers.HttpServer
         public NoEventsMethod NoEvents;
         public RequestMethod Request;
         public UUID Id;
-        public PollServiceEventArgs(RequestMethod pRequest, HasEventsMethod pHasEvents, GetEventsMethod pGetEvents, NoEventsMethod pNoEvents,UUID pId)
+
+        public PollServiceEventArgs(
+            RequestMethod pRequest,
+            HasEventsMethod pHasEvents, GetEventsMethod pGetEvents, NoEventsMethod pNoEvents,
+            UUID pId)
         {
             Request = pRequest;
             HasEvents = pHasEvents;
@@ -53,4 +58,4 @@ namespace OpenSim.Framework.Servers.HttpServer
             Id = pId;
         }
     }
-}
+}
\ No newline at end of file
diff --git a/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs b/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs
index 553a7eb..723530a 100644
--- a/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs
+++ b/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs
@@ -31,7 +31,6 @@ using OpenMetaverse;
 
 namespace OpenSim.Framework.Servers.HttpServer
 {
-
     public class PollServiceHttpRequest
     {
         public readonly PollServiceEventArgs PollServiceArgs;
@@ -39,7 +38,9 @@ namespace OpenSim.Framework.Servers.HttpServer
         public readonly IHttpRequest Request;
         public readonly int RequestTime;
         public readonly UUID RequestID;
-        public PollServiceHttpRequest(PollServiceEventArgs pPollServiceArgs, IHttpClientContext pHttpContext, IHttpRequest pRequest)
+
+        public PollServiceHttpRequest(
+            PollServiceEventArgs pPollServiceArgs, IHttpClientContext pHttpContext, IHttpRequest pRequest)
         {
             PollServiceArgs = pPollServiceArgs;
             HttpContext = pHttpContext;
@@ -48,4 +49,4 @@ namespace OpenSim.Framework.Servers.HttpServer
             RequestID = UUID.Random();
         }
     }
-}
+}
\ No newline at end of file
diff --git a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
index 61afc76..5c05500 100644
--- a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
+++ b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
@@ -64,17 +64,25 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
         public string uri;
     }
 
+    /// <summary>
+    /// This module provides external URLs for in-world scripts.
+    /// </summary>
     public class UrlModule : ISharedRegionModule, IUrlModule
     {
         private static readonly ILog m_log =
                 LogManager.GetLogger(
                 MethodBase.GetCurrentMethod().DeclaringType);
 
-        private Dictionary<UUID, UrlData> m_RequestMap =
-                new Dictionary<UUID, UrlData>();
+        /// <summary>
+        /// 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>
+        private Dictionary<UUID, UrlData> m_RequestMap = new Dictionary<UUID, UrlData>();
 
-        private Dictionary<string, UrlData> m_UrlMap =
-                new Dictionary<string, UrlData>();
+        /// <summary>
+        /// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the full URL
+        /// </summary>
+        private Dictionary<string, UrlData> m_UrlMap = new Dictionary<string, UrlData>();
 
         /// <summary>
         /// Maximum number of external urls that can be set up by this module.
@@ -224,7 +232,6 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                 urlData.urlcode = urlcode;
                 urlData.requests = new Dictionary<UUID, RequestData>();
 
-                
                 m_UrlMap[url] = urlData;
                 
                 string uri = "/lslhttps/" + urlcode.ToString() + "/";
@@ -286,7 +293,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
         {
             if (m_RequestMap.ContainsKey(requestId))
             {
-                UrlData urlData=m_RequestMap[requestId];
+                UrlData urlData = m_RequestMap[requestId];
                 string value;
                 if (urlData.requests[requestId].headers.TryGetValue(header,out value))
                     return value;
@@ -295,6 +302,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
             {
                 m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId);
             }
+
             return String.Empty;
         }
 
@@ -339,6 +347,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                     {
                         RemoveUrl(url.Value);
                         removeURLs.Add(url.Key);
+
                         foreach (UUID req in url.Value.requests.Keys)
                             m_RequestMap.Remove(req);
                     }
@@ -349,20 +358,31 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
             }
         }
 
-
         private void RemoveUrl(UrlData data)
         {
-            m_HttpServer.RemoveHTTPHandler("", "/lslhttp/"+data.urlcode.ToString()+"/");
+            m_HttpServer.RemoveHTTPHandler("", "/lslhttp/" + data.urlcode.ToString() + "/");
         }
 
         private Hashtable NoEvents(UUID requestID, UUID sessionID)
         {
             Hashtable response = new Hashtable();
             UrlData url;
+
             lock (m_RequestMap)
             {
+                // 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
+                // for the request ID.
                 if (!m_RequestMap.ContainsKey(requestID))
+                {
+                    response["int_response_code"] = 404;
+                    response["str_response_string"] = "";
+                    response["keepalive"] = false;
+                    response["reusecontext"] = false;
+
                     return response;
+                }
+
                 url = m_RequestMap[requestID];
             }
 
@@ -384,53 +404,57 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                 return response;
             }
 
-            
             return response;
         }
 
         private bool HasEvents(UUID requestID, UUID sessionID)
         {
-            UrlData url=null;
+            UrlData url = null;
             
             lock (m_RequestMap)
             {
+                // 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
+                // when it detects that the URL has been removed.  The poller, however, will continue to ask for
+                // events for that request, so here we will signal that there are events and in GetEvents we will
+                // return a 404.
                 if (!m_RequestMap.ContainsKey(requestID))
                 {
-                    return false;
+                    return true;
                 }
+
                 url = m_RequestMap[requestID];
                 if (!url.requests.ContainsKey(requestID))
                 {
-                    return false;
+                    return true;
                 }
             }
 
-            if (System.Environment.TickCount-url.requests[requestID].startTime>25000)
+            // Trigger return of timeout response.
+            if (System.Environment.TickCount - url.requests[requestID].startTime > 25000)
             {
                 return true;
             }
 
-            if (url.requests[requestID].requestDone)
-                return true;
-            else
-                return false;
-
+            return url.requests[requestID].requestDone;
         }
+
         private Hashtable GetEvents(UUID requestID, UUID sessionID, string request)
         {
-             UrlData url = null;
+            UrlData url = null;
             RequestData requestData = null;
 
             lock (m_RequestMap)
             {
                 if (!m_RequestMap.ContainsKey(requestID))
-                    return NoEvents(requestID,sessionID);
+                    return NoEvents(requestID, sessionID);
+
                 url = m_RequestMap[requestID];
                 requestData = url.requests[requestID];
             }
 
             if (!requestData.requestDone)
-                return NoEvents(requestID,sessionID);
+                return NoEvents(requestID, sessionID);
             
             Hashtable response = new Hashtable();
 
@@ -443,6 +467,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                 response["reusecontext"] = false;
                 return response;
             }
+
             //put response
             response["int_response_code"] = requestData.responseCode;
             response["str_response_string"] = requestData.responseBody;
@@ -459,6 +484,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
 
             return response;
         }
+
         public void HttpRequestHandler(UUID requestID, Hashtable request)
         {
             lock (request)
@@ -483,11 +509,22 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
 
                     pathInfo = uri.Substring(pos3);
 
-                    UrlData url = null;
-                    if (!is_ssl)
-                        url = m_UrlMap["http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp];
-                    else
-                        url = m_UrlMap["https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp];
+                    UrlData urlData = null;
+
+                    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;
+
+                        // 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 
@@ -520,11 +557,10 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                                     queryString = queryString + key + "=" + val + "&";
                                 }
                             }
+
                             if (queryString.Length > 1)
                                 queryString = queryString.Substring(0, queryString.Length - 1);
-
                         }
-
                     }
 
                     //if this machine is behind DNAT/port forwarding, currently this is being
@@ -532,26 +568,28 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
                     requestData.headers["x-remote-ip"] = requestData.headers["remote_addr"];
                     requestData.headers["x-path-info"] = pathInfo;
                     requestData.headers["x-query-string"] = queryString;
-                    requestData.headers["x-script-url"] = url.url;
+                    requestData.headers["x-script-url"] = urlData.url;
 
                     //requestData.ev = new ManualResetEvent(false);
-                    lock (url.requests)
+                    lock (urlData.requests)
                     {
-                        url.requests.Add(requestID, requestData);
+                        urlData.requests.Add(requestID, requestData);
                     }
+
                     lock (m_RequestMap)
                     {
-                        //add to request map
-                        m_RequestMap.Add(requestID, url);
+                        m_RequestMap.Add(requestID, urlData);
                     }
 
-                    url.engine.PostScriptEvent(url.itemID, "http_request", new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
+                    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)
                 {