* Fixed a bug where clients were being added to ClientManager twice

* Changed the ClientManager interface to reduce potential errors with duplicate or mismatched keys
* Added IClientAPI.RemoteEndPoint, which can (hopefully) eventually replace IClientAPI.CircuitCode
* Changed the order of operations during client shutdown

5 files changed, 54 insertions, 59 deletions

diff --git a/OpenSim/Framework/ClientManager.cs b/OpenSim/Framework/ClientManager.cs
index 4edfabe..aefe425 100644
--- a/OpenSim/Framework/ClientManager.cs
+++ b/OpenSim/Framework/ClientManager.cs
@@ -97,19 +97,17 @@ namespace OpenSim.Framework
         /// Add a client reference to the collection if it does not already
         /// exist
         /// </summary>
-        /// <param name="key">UUID of the client</param>
-        /// <param name="key2">Remote endpoint of the client</param>
         /// <param name="value">Reference to the client object</param>
         /// <returns>True if the client reference was successfully added,
         /// otherwise false if the given key already existed in the collection</returns>
-        public bool Add(UUID key, IPEndPoint key2, IClientAPI value)
+        public bool Add(IClientAPI value)
         {
             lock (m_writeLock)
             {
-                if (!m_dict.ContainsKey(key) && !m_dict2.ContainsKey(key2))
+                if (!m_dict.ContainsKey(value.AgentId) && !m_dict2.ContainsKey(value.RemoteEndPoint))
                 {
-                    m_dict = m_dict.Add(key, value);
-                    m_dict2 = m_dict2.Add(key2, value);
+                    m_dict = m_dict.Add(value.AgentId, value);
+                    m_dict2 = m_dict2.Add(value.RemoteEndPoint, value);
 
                     return true;
                 }
@@ -123,14 +121,16 @@ namespace OpenSim.Framework
         /// <summary>
         /// Remove a client from the collection
         /// </summary>
-        /// <param name="key">UUID of the client</param>
-        /// <param name="key2">Remote endpoint of the client</param>
-        public void Remove(UUID key, IPEndPoint key2)
+        /// <param name="value">Reference to the client object</param>
+        public void Remove(IClientAPI value)
         {
             lock (m_writeLock)
             {
-                m_dict = m_dict.Delete(key);
-                m_dict2 = m_dict2.Delete(key2);
+                if (m_dict.ContainsKey(value.AgentId))
+                    m_dict = m_dict.Delete(value.AgentId);
+
+                if (m_dict2.ContainsKey(value.RemoteEndPoint))
+                    m_dict2 = m_dict2.Delete(value.RemoteEndPoint);
             }
         }
 
diff --git a/OpenSim/Framework/IClientAPI.cs b/OpenSim/Framework/IClientAPI.cs
index 99ea0d5..29846f5 100644
--- a/OpenSim/Framework/IClientAPI.cs
+++ b/OpenSim/Framework/IClientAPI.cs
@@ -561,6 +561,8 @@ namespace OpenSim.Framework
         // [Obsolete("LLClientView Specific - Circuits are unique to LLClientView")]
         uint CircuitCode { get; }
 
+        IPEndPoint RemoteEndPoint { get; }
+
         event GenericMessage OnGenericMessage;
 
         // [Obsolete("LLClientView Specific - Replace with more bare-bones arguments.")]
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs b/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs
index 86d0112..aecb362 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs
@@ -411,38 +411,34 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 "[CLIENT]: Close has been called for {0} attached to scene {1}",
                 Name, m_scene.RegionInfo.RegionName);
 
-            // Remove ourselves from the scene
-            m_scene.ClientManager.Remove(m_agentId, m_udpClient.RemoteEndPoint);
+            // Send the STOP packet
+            DisableSimulatorPacket disable = (DisableSimulatorPacket)PacketPool.Instance.GetPacket(PacketType.DisableSimulator);
+            OutPacket(disable, ThrottleOutPacketType.Unknown);
+
+            IsActive = false;
 
+            // Shutdown the image manager
             if (m_imageManager != null)
-            {
                 m_imageManager.Close();
-                m_imageManager = null;
-            }
-
-            if (m_udpServer != null)
-            {
-                m_udpServer.Flush();
-            }
 
+            // Fire the callback for this connection closing
             if (OnConnectionClosed != null)
                 OnConnectionClosed(this);
 
-            CloseCleanup();
-        }
+            // Flush all of the packets out of the UDP server for this client
+            if (m_udpServer != null)
+                m_udpServer.Flush(m_udpClient);
 
-        private void CloseCleanup()
-        {
+            // Remove ourselves from the scene
             m_scene.RemoveClient(AgentId);
+            m_scene.ClientManager.Remove(this);
 
             //m_log.InfoFormat("[CLIENTVIEW] Memory pre  GC {0}", System.GC.GetTotalMemory(false));
+            //GC.Collect();
             //m_log.InfoFormat("[CLIENTVIEW] Memory post GC {0}", System.GC.GetTotalMemory(true));
 
-            // Send the STOP packet
-            DisableSimulatorPacket disable = (DisableSimulatorPacket)PacketPool.Instance.GetPacket(PacketType.DisableSimulator);
-            OutPacket(disable, ThrottleOutPacketType.Unknown);
-
-            Thread.Sleep(2000);
+            // FIXME: Is this still necessary?
+            //Thread.Sleep(2000);
 
             // Shut down timers. Thread Context of this method is murky.   Lock all timers
             if (m_avatarTerseUpdateTimer.Enabled)
@@ -459,8 +455,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             // We need to do this over grid communications
             //m_scene.CloseAllAgents(CircuitCode);
 
-            IsActive = false;
-
             m_avatarTerseUpdateTimer.Dispose();
             m_primTerseUpdateTimer.Dispose();
             m_primFullUpdateTimer.Dispose();
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLImageManager.cs b/OpenSim/Region/ClientStack/LindenUDP/LLImageManager.cs
index 343f537..8469ba6 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLImageManager.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLImageManager.cs
@@ -167,39 +167,39 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             J2KImage imagereq;
             int numCollected = 0;
 
-            //lock (m_syncRoot)
-            //{
-                m_lastloopprocessed = DateTime.Now.Ticks;
-
-                // This can happen during Close()
-                if (m_client == null)
-                    return false;
-                
-                while ((imagereq = GetHighestPriorityImage()) != null)
+            m_lastloopprocessed = DateTime.Now.Ticks;
+
+            // This can happen during Close()
+            if (m_client == null)
+                return false;
+
+            while ((imagereq = GetHighestPriorityImage()) != null)
+            {
+                if (imagereq.IsDecoded == true)
                 {
-                    if (imagereq.IsDecoded == true)
-                    {
-                        ++numCollected;
+                    ++numCollected;
 
-                        if (imagereq.SendPackets(m_client, maxpack))
-                        {
-                            // Send complete. Destroy any knowledge of this transfer
-                            RemoveImageFromQueue(imagereq);
-                        }
+                    if (imagereq.SendPackets(m_client, maxpack))
+                    {
+                        // Send complete. Destroy any knowledge of this transfer
+                        RemoveImageFromQueue(imagereq);
                     }
-
-                    if (numCollected == count)
-                        break;
                 }
-            //}
+
+                if (numCollected == count)
+                    break;
+            }
 
             return m_priorityQueue.Count > 0;
         }
 
-        //Faux destructor
+        /// <summary>
+        /// Faux destructor
+        /// </summary>
         public void Close()
         {
             m_shuttingdown = true;
+            m_priorityQueue = null;
             m_j2kDecodeModule = null;
             m_assetCache = null;
             m_client = null;
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
index 8ec143a..22c275c 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
@@ -390,7 +390,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             }
         }
 
-        public void Flush()
+        public void Flush(LLUDPClient udpClient)
         {
             // FIXME: Implement?
         }
@@ -645,15 +645,14 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 client.OnLogout += LogoutHandler;
                 client.OnConnectionClosed += ConnectionClosedHandler;
 
-                m_scene.ClientManager.Add(agentID, remoteEndPoint, client);
-
                 // Start the IClientAPI
-                m_scene.ClientManager.Add(agentID, remoteEndPoint, client);
+                m_scene.ClientManager.Add(client);
                 client.Start();
             }
             else
             {
-                m_log.Debug("[LLUDPSERVER]: Ignoring a repeated UseCircuitCode from " + udpClient.AgentID);
+                m_log.WarnFormat("[LLUDPSERVER]: Ignoring a repeated UseCircuitCode from {0} at {1} for circuit {2}",
+                    udpClient.AgentID, remoteEndPoint, circuitCode);
             }
         }