7 files changed, 195 insertions, 195 deletions

diff --git a/OpenSim/Region/ClientStack/LindenUDP/IncomingPacket.cs b/OpenSim/Region/ClientStack/LindenUDP/IncomingPacket.cs
index dc0d62a..90b3ede 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/IncomingPacket.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/IncomingPacket.cs
@@ -32,11 +32,26 @@ using OpenMetaverse.Packets;
 
 namespace OpenSim.Region.ClientStack.LindenUDP
 {
-    public struct IncomingPacket
+    /// <summary>
+    /// Holds a reference to a <seealso cref="LLUDPClient"/> and a <seealso cref="Packet"/>
+    /// for incoming packets
+    /// </summary>
+    public sealed class IncomingPacket
     {
         /// <summary>Client this packet came from</summary>
         public LLUDPClient Client;
         /// <summary>Packet data that has been received</summary>
         public Packet Packet;
+
+        /// <summary>
+        /// Default constructor
+        /// </summary>
+        /// <param name="client">Reference to the client this packet came from</param>
+        /// <param name="packet">Packet data</param>
+        public IncomingPacket(LLUDPClient client, Packet packet)
+        {
+            Client = client;
+            Packet = packet;
+        }
     }
 }
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLPacketThrottle.cs b/OpenSim/Region/ClientStack/LindenUDP/LLPacketThrottle.cs
deleted file mode 100644
index 52effc5..0000000
--- a/OpenSim/Region/ClientStack/LindenUDP/LLPacketThrottle.cs
+++ /dev/null
@@ -1,128 +0,0 @@
-/*
- * Copyright (c) Contributors, http://opensimulator.org/
- * See CONTRIBUTORS.TXT for a full list of copyright holders.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in the
- *       documentation and/or other materials provided with the distribution.
- *     * Neither the name of the OpenSimulator Project nor the
- *       names of its contributors may be used to endorse or promote products
- *       derived from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL THE CONTRIBUTORS BE LIABLE FOR ANY
- * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
- * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-namespace OpenSim.Region.ClientStack.LindenUDP
-{
-    public class LLPacketThrottle
-    {
-        private readonly int m_maxAllowableThrottle;
-        private readonly int m_minAllowableThrottle;
-        private int m_currentThrottle;
-        private const int m_throttleTimeDivisor = 7;
-        private int m_currentBitsSent;
-        private int m_throttleBits;
-        
-        /// <value>
-        /// Value with which to multiply all the throttle fields
-        /// </value>
-        private float m_throttleMultiplier;
-        
-        public int Max
-        {
-            get { return m_maxAllowableThrottle; }
-        }
-
-        public int Min
-        {
-            get { return m_minAllowableThrottle; }
-        }
-        
-        public int Current
-        {
-            get { return m_currentThrottle; }
-        }
-
-        /// <summary>
-        /// Constructor.
-        /// </summary>
-        /// <param name="min"></param>
-        /// <param name="max"></param>
-        /// <param name="throttle"></param>
-        /// <param name="throttleMultiplier">
-        /// A parameter that's ends up multiplying all throttle settings.  An alternative solution would have been 
-        /// to multiply all the parameters by this before giving them to the constructor.  But doing it this way
-        /// represents the fact that the multiplier is a hack that pumps data to clients much faster than the actual
-        /// settings that we are given.
-        /// </param>
-        public LLPacketThrottle(int min, int max, int throttle, float throttleMultiplier)
-        {
-            m_throttleMultiplier = throttleMultiplier;
-            m_maxAllowableThrottle = max;
-            m_minAllowableThrottle = min;
-            m_currentThrottle = throttle;
-            m_currentBitsSent = 0;
-
-            CalcBits();
-        }
-
-        /// <summary>
-        /// Calculate the actual throttle required.
-        /// </summary>
-        private void CalcBits()
-        {
-            m_throttleBits = (int)((float)m_currentThrottle * m_throttleMultiplier / (float)m_throttleTimeDivisor);
-        }
-
-        public void Reset()
-        {
-            m_currentBitsSent = 0;
-        }
-
-        public bool UnderLimit()
-        {
-            return m_currentBitsSent < m_throttleBits;
-        }
-        
-        public int AddBytes(int bytes)
-        {
-            m_currentBitsSent += bytes * 8;
-            return m_currentBitsSent;
-        }
-
-        public int Throttle
-        {
-            get { return m_currentThrottle; }
-            set
-            {
-                if (value < m_minAllowableThrottle)
-                {
-                    m_currentThrottle = m_minAllowableThrottle;
-                }
-                else if (value > m_maxAllowableThrottle)
-                {
-                    m_currentThrottle = m_maxAllowableThrottle;
-                }
-                else
-                {
-                    m_currentThrottle = value;
-                }
-
-                CalcBits();
-            }
-        }
-    }
-}
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLUDPClient.cs b/OpenSim/Region/ClientStack/LindenUDP/LLUDPClient.cs
index f2e76d3..871e8e8 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLUDPClient.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLUDPClient.cs
@@ -33,16 +33,40 @@ using OpenMetaverse;
 
 namespace OpenSim.Region.ClientStack.LindenUDP
 {
+    #region Delegates
+
+    /// <summary>
+    /// Fired when updated networking stats are produced for this client
+    /// </summary>
+    /// <param name="inPackets">Number of incoming packets received since this
+    /// event was last fired</param>
+    /// <param name="outPackets">Number of outgoing packets sent since this
+    /// event was last fired</param>
+    /// <param name="unAckedBytes">Current total number of bytes in packets we
+    /// are waiting on ACKs for</param>
     public delegate void PacketStats(int inPackets, int outPackets, int unAckedBytes);
+    /// <summary>
+    /// Fired when the queue for a packet category is empty. This event can be
+    /// hooked to put more data on the empty queue
+    /// </summary>
+    /// <param name="category">Category of the packet queue that is empty</param>
     public delegate void QueueEmpty(ThrottleOutPacketType category);
 
-    public class LLUDPClient
+    #endregion Delegates
+
+    /// <summary>
+    /// Tracks state for a client UDP connection and provides client-specific methods
+    /// </summary>
+    public sealed class LLUDPClient
     {
         /// <summary>The number of packet categories to throttle on. If a throttle category is added
         /// or removed, this number must also change</summary>
         const int THROTTLE_CATEGORY_COUNT = 7;
 
+        /// <summary>Fired when updated networking stats are produced for this client</summary>
         public event PacketStats OnPacketStats;
+        /// <summary>Fired when the queue for a packet category is empty. This event can be
+        /// hooked to put more data on the empty queue</summary>
         public event QueueEmpty OnQueueEmpty;
 
         /// <summary>AgentID for this client</summary>
@@ -115,6 +139,16 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// <summary>A reference to the LLUDPServer that is managing this client</summary>
         private readonly LLUDPServer udpServer;
 
+        /// <summary>
+        /// Default constructor
+        /// </summary>
+        /// <param name="server">Reference to the UDP server this client is connected to</param>
+        /// <param name="rates">Default throttling rates and maximum throttle limits</param>
+        /// <param name="parentThrottle">Parent HTB (hierarchical token bucket)
+        /// that the child throttles will be governed by</param>
+        /// <param name="circuitCode">Circuit code for this connection</param>
+        /// <param name="agentID">AgentID for the connected agent</param>
+        /// <param name="remoteEndPoint">Remote endpoint for this connection</param>
         public LLUDPClient(LLUDPServer server, ThrottleRates rates, TokenBucket parentThrottle, uint circuitCode, UUID agentID, IPEndPoint remoteEndPoint)
         {
             udpServer = server;
@@ -144,14 +178,24 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             RTO = 3000;
         }
 
+        /// <summary>
+        /// Shuts down this client connection
+        /// </summary>
         public void Shutdown()
         {
+            // TODO: Do we need to invalidate the circuit?
             IsConnected = false;
         }
 
+        /// <summary>
+        /// Gets information about this client connection
+        /// </summary>
+        /// <returns>Information about the client connection</returns>
         public ClientInfo GetClientInfo()
         {
-            // TODO: This data structure is wrong in so many ways
+            // TODO: This data structure is wrong in so many ways. Locking and copying the entire lists
+            // of pending and needed ACKs for every client every time some method wants information about
+            // this connection is a recipe for poor performance
             ClientInfo info = new ClientInfo();
             info.pendingAcks = new Dictionary<uint, uint>();
             info.needAck = new Dictionary<uint, byte[]>();
@@ -169,8 +213,16 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             return info;
         }
 
+        /// <summary>
+        /// Modifies the UDP throttles
+        /// </summary>
+        /// <param name="info">New throttling values</param>
         public void SetClientInfo(ClientInfo info)
         {
+            // TODO: Allowing throttles to be manually set from this function seems like a reasonable
+            // idea. On the other hand, letting external code manipulate our ACK accounting is not
+            // going to happen
+            throw new NotImplementedException();
         }
 
         public string GetStats()
diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
index 38890da..c0a84a8 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/LLUDPServer.cs
@@ -41,7 +41,10 @@ using OpenMetaverse;
 
 namespace OpenSim.Region.ClientStack.LindenUDP
 {
-    public class LLUDPServerShim : IClientNetworkServer
+    /// <summary>
+    /// A shim around LLUDPServer that implements the IClientNetworkServer interface
+    /// </summary>
+    public sealed class LLUDPServerShim : IClientNetworkServer
     {
         LLUDPServer m_udpServer;
 
@@ -80,6 +83,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         }
     }
 
+    /// <summary>
+    /// The LLUDP server for a region. This handles incoming and outgoing
+    /// packets for all UDP connections to the region
+    /// </summary>
     public class LLUDPServer : UDPBase
     {
         private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
@@ -152,6 +159,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 
         public new void Stop()
         {
+            m_log.Info("[LLUDPSERVER]: Shutting down the LLUDP server for " + m_scene.RegionInfo.RegionName);
             base.Stop();
         }
 
@@ -591,11 +599,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             if (packet.Type != PacketType.PacketAck)
             {
                 // Inbox insertion
-                IncomingPacket incomingPacket;
-                incomingPacket.Client = client;
-                incomingPacket.Packet = packet;
-
-                packetInbox.Enqueue(incomingPacket);
+                packetInbox.Enqueue(new IncomingPacket(client, packet));
             }
         }
 
@@ -683,7 +687,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             // on to en-US to avoid number parsing issues
             Culture.SetCurrentCulture();
 
-            IncomingPacket incomingPacket = default(IncomingPacket);
+            IncomingPacket incomingPacket = null;
 
             while (base.IsRunning)
             {
@@ -696,59 +700,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             packetInbox.Clear();
         }
 
-        private void ProcessInPacket(object state)
-        {
-            IncomingPacket incomingPacket = (IncomingPacket)state;
-            Packet packet = incomingPacket.Packet;
-            LLUDPClient client = incomingPacket.Client;
-
-            if (packet != null && client != null)
-            {
-                try
-                {
-                    client.ClientAPI.ProcessInPacket(packet);
-                }
-                catch (ThreadAbortException)
-                {
-                    throw;
-                }
-                catch (Exception e)
-                {
-                    if (StatsManager.SimExtraStats != null)
-                        StatsManager.SimExtraStats.AddAbnormalClientThreadTermination();
-
-                    // Don't let a failure in an individual client thread crash the whole sim.
-                    m_log.ErrorFormat("[LLUDPSERVER]: Client thread for {0} crashed. Logging them out", client.AgentID);
-                    m_log.Error(e.Message, e);
-
-                    try
-                    {
-                        // Make an attempt to alert the user that their session has crashed
-                        AgentAlertMessagePacket alert = client.ClientAPI.BuildAgentAlertPacket(
-                            "Unfortunately the session for this client on the server has crashed.\n" +
-                            "Any further actions taken will not be processed.\n" +
-                            "Please relog", true);
-
-                        SendPacket(client, alert, ThrottleOutPacketType.Unknown, false);
-
-                        // TODO: There may be a better way to do this. Perhaps kick? Not sure this propogates notifications to
-                        // listeners yet, though.
-                        client.ClientAPI.SendLogoutPacket();
-                        RemoveClient(client.ClientAPI);
-                    }
-                    catch (ThreadAbortException)
-                    {
-                        throw;
-                    }
-                    catch (Exception e2)
-                    {
-                        m_log.Error("[LLUDPSERVER]: Further exception thrown on forced session logout for " + client.AgentID);
-                        m_log.Error(e2.Message, e2);
-                    }
-                }
-            }
-        }
-
         private void OutgoingPacketHandler()
         {
             // Set this culture for the thread that outgoing packets are sent
@@ -812,6 +763,38 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             }
         }
 
+        private void ProcessInPacket(object state)
+        {
+            IncomingPacket incomingPacket = (IncomingPacket)state;
+            Packet packet = incomingPacket.Packet;
+            LLUDPClient client = incomingPacket.Client;
+
+            // Sanity check
+            if (packet == null || client == null || client.ClientAPI == null)
+            {
+                m_log.WarnFormat("[LLUDPSERVER]: Processing a packet with incomplete state. Packet=\"{0}\", Client=\"{1}\", Client.ClientAPI=\"{2}\"",
+                    packet, client, (client != null) ? client.ClientAPI : null);
+            }
+
+            try
+            {
+                // Process this packet
+                client.ClientAPI.ProcessInPacket(packet);
+            }
+            catch (ThreadAbortException)
+            {
+                // If something is trying to abort the packet processing thread, take that as a hint that it's time to shut down
+                m_log.Info("[LLUDPSERVER]: Caught a thread abort, shutting down the LLUDP server");
+                Stop();
+            }
+            catch (Exception e)
+            {
+                // Don't let a failure in an individual client thread crash the whole sim.
+                m_log.ErrorFormat("[LLUDPSERVER]: Client packet handler for {0} for packet {1} threw an exception", client.AgentID, packet.Type);
+                m_log.Error(e.Message, e);
+            }
+        }
+
         private void LogoutHandler(IClientAPI client)
         {
             client.SendLogoutPacket();
diff --git a/OpenSim/Region/ClientStack/LindenUDP/OutgoingPacket.cs b/OpenSim/Region/ClientStack/LindenUDP/OutgoingPacket.cs
index 69b0c5f..1a1a1cb 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/OutgoingPacket.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/OutgoingPacket.cs
@@ -31,6 +31,13 @@ using OpenMetaverse;
 
 namespace OpenSim.Region.ClientStack.LindenUDP
 {
+    /// <summary>
+    /// Holds a reference to the <seealso cref="LLUDPClient"/> this packet is
+    /// destined for, along with the serialized packet data, sequence number
+    /// (if this is a resend), number of times this packet has been resent,
+    /// the time of the last resend, and the throttling category for this
+    /// packet
+    /// </summary>
     public sealed class OutgoingPacket
     {
         /// <summary>Client this packet is destined for</summary>
@@ -46,6 +53,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// <summary>Category this packet belongs to</summary>
         public ThrottleOutPacketType Category;
 
+        /// <summary>
+        /// Default constructor
+        /// </summary>
+        /// <param name="client">Reference to the client this packet is destined for</param>
+        /// <param name="buffer">Serialized packet data. If the flags or sequence number
+        /// need to be updated, they will be injected directly into this binary buffer</param>
+        /// <param name="category">Throttling category for this packet</param>
         public OutgoingPacket(LLUDPClient client, UDPPacketBuffer buffer, ThrottleOutPacketType category)
         {
             Client = client;
diff --git a/OpenSim/Region/ClientStack/LindenUDP/ThrottleRates.cs b/OpenSim/Region/ClientStack/LindenUDP/ThrottleRates.cs
index ffa20d5..858a03c 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/ThrottleRates.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/ThrottleRates.cs
@@ -30,24 +30,47 @@ using Nini.Config;
 
 namespace OpenSim.Region.ClientStack.LindenUDP
 {
+    /// <summary>
+    /// Holds drip rates and maximum burst rates for throttling with hierarchical
+    /// token buckets. The maximum burst rates set here are hard limits and can
+    /// not be overridden by client requests
+    /// </summary>
     public sealed class ThrottleRates
     {
+        /// <summary>Drip rate for resent packets</summary>
         public int Resend;
+        /// <summary>Drip rate for terrain packets</summary>
         public int Land;
+        /// <summary>Drip rate for wind packets</summary>
         public int Wind;
+        /// <summary>Drip rate for cloud packets</summary>
         public int Cloud;
+        /// <summary>Drip rate for task (state and transaction) packets</summary>
         public int Task;
+        /// <summary>Drip rate for texture packets</summary>
         public int Texture;
+        /// <summary>Drip rate for asset packets</summary>
         public int Asset;
 
+        /// <summary>Maximum burst rate for resent packets</summary>
         public int ResendLimit;
+        /// <summary>Maximum burst rate for land packets</summary>
         public int LandLimit;
+        /// <summary>Maximum burst rate for wind packets</summary>
         public int WindLimit;
+        /// <summary>Maximum burst rate for cloud packets</summary>
         public int CloudLimit;
+        /// <summary>Maximum burst rate for task (state and transaction) packets</summary>
         public int TaskLimit;
+        /// <summary>Maximum burst rate for texture packets</summary>
         public int TextureLimit;
+        /// <summary>Maximum burst rate for asset packets</summary>
         public int AssetLimit;
 
+        /// <summary>
+        /// Default constructor
+        /// </summary>
+        /// <param name="config">Config source to load defaults from</param>
         public ThrottleRates(IConfigSource config)
         {
             try
diff --git a/OpenSim/Region/ClientStack/LindenUDP/UnackedPacketCollection.cs b/OpenSim/Region/ClientStack/LindenUDP/UnackedPacketCollection.cs
index 16c0035..b7df84d 100644
--- a/OpenSim/Region/ClientStack/LindenUDP/UnackedPacketCollection.cs
+++ b/OpenSim/Region/ClientStack/LindenUDP/UnackedPacketCollection.cs
@@ -32,19 +32,34 @@ using OpenMetaverse;
 
 namespace OpenSim.Region.ClientStack.LindenUDP
 {
+    /// <summary>
+    /// Special collection that is optimized for tracking unacknowledged packets
+    /// </summary>
     public sealed class UnackedPacketCollection
     {
+        /// <summary>Synchronization primitive. A lock must be acquired on this
+        /// object before calling any of the unsafe methods</summary>
         public object SyncRoot = new object();
 
-        SortedDictionary<uint, OutgoingPacket> packets;
+        /// <summary>Holds the actual unacked packet data, sorted by sequence number</summary>
+        private SortedDictionary<uint, OutgoingPacket> packets = new SortedDictionary<uint, OutgoingPacket>();
 
+        /// <summary>Gets the total number of unacked packets</summary>
         public int Count { get { return packets.Count; } }
 
+        /// <summary>
+        /// Default constructor
+        /// </summary>
         public UnackedPacketCollection()
         {
-            packets = new SortedDictionary<uint, OutgoingPacket>();
         }
 
+        /// <summary>
+        /// Add an unacked packet to the collection
+        /// </summary>
+        /// <param name="packet">Packet that is awaiting acknowledgement</param>
+        /// <returns>True if the packet was successfully added, false if the
+        /// packet already existed in the collection</returns>
         public bool Add(OutgoingPacket packet)
         {
             lock (SyncRoot)
@@ -58,11 +73,24 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             }
         }
 
+        /// <summary>
+        /// Removes a packet from the collection without attempting to obtain a
+        /// lock first
+        /// </summary>
+        /// <param name="sequenceNumber">Sequence number of the packet to remove</param>
+        /// <returns>True if the packet was found and removed, otherwise false</returns>
         public bool RemoveUnsafe(uint sequenceNumber)
         {
             return packets.Remove(sequenceNumber);
         }
 
+        /// <summary>
+        /// Removes a packet from the collection without attempting to obtain a
+        /// lock first
+        /// </summary>
+        /// <param name="sequenceNumber">Sequence number of the packet to remove</param>
+        /// <param name="packet">Returns the removed packet</param>
+        /// <returns>True if the packet was found and removed, otherwise false</returns>
         public bool RemoveUnsafe(uint sequenceNumber, out OutgoingPacket packet)
         {
             if (packets.TryGetValue(sequenceNumber, out packet))
@@ -74,6 +102,11 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             return false;
         }
 
+        /// <summary>
+        /// Gets the packet with the lowest sequence number
+        /// </summary>
+        /// <returns>The packet with the lowest sequence number, or null if the
+        /// collection is empty</returns>
         public OutgoingPacket GetOldest()
         {
             lock (SyncRoot)
@@ -83,7 +116,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             }
         }
 
-        public List<OutgoingPacket> GetExpiredPackets(int timeout)
+        /// <summary>
+        /// Returns a list of all of the packets with a TickCount older than
+        /// the specified timeout
+        /// </summary>
+        /// <param name="timeoutMS">Number of ticks (milliseconds) before a
+        /// packet is considered expired</param>
+        /// <returns>A list of all expired packets according to the given
+        /// expiration timeout</returns>
+        public List<OutgoingPacket> GetExpiredPackets(int timeoutMS)
         {
             List<OutgoingPacket> expiredPackets = null;
 
@@ -95,7 +136,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                     if (packet.TickCount == 0)
                         continue;
 
-                    if (now - packet.TickCount >= timeout)
+                    if (now - packet.TickCount >= timeoutMS)
                     {
                         if (expiredPackets == null)
                             expiredPackets = new List<OutgoingPacket>();