From 61b537215328499155c58f46e6338d459aba87ec Mon Sep 17 00:00:00 2001 From: John Hurliman Date: Tue, 6 Oct 2009 12:13:16 -0700 Subject: * Added missing references to prebuild.xml and commented out the LindenUDP tests until a new test harness is written * Clients are no longer disconnected when a packet handler crashes. We'll see how this works out in practice * Added documentation and cleanup, getting ready for the first public push * Deleted an old LLUDP file --- .../Region/ClientStack/LindenUDP/IncomingPacket.cs | 17 ++- .../ClientStack/LindenUDP/LLPacketThrottle.cs | 128 --------------------- .../Region/ClientStack/LindenUDP/LLUDPClient.cs | 56 ++++++++- .../Region/ClientStack/LindenUDP/LLUDPServer.cs | 103 +++++++---------- .../Region/ClientStack/LindenUDP/OutgoingPacket.cs | 14 +++ .../Region/ClientStack/LindenUDP/ThrottleRates.cs | 23 ++++ .../LindenUDP/UnackedPacketCollection.cs | 49 +++++++- prebuild.xml | 4 + 8 files changed, 199 insertions(+), 195 deletions(-) delete mode 100644 OpenSim/Region/ClientStack/LindenUDP/LLPacketThrottle.cs 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 + /// + /// Holds a reference to a and a + /// for incoming packets + /// + public sealed class IncomingPacket { /// Client this packet came from public LLUDPClient Client; /// Packet data that has been received public Packet Packet; + + /// + /// Default constructor + /// + /// Reference to the client this packet came from + /// Packet data + 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 with which to multiply all the throttle fields - /// - 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; } - } - - /// - /// Constructor. - /// - /// - /// - /// - /// - /// 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. - /// - 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(); - } - - /// - /// Calculate the actual throttle required. - /// - 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 + + /// + /// Fired when updated networking stats are produced for this client + /// + /// Number of incoming packets received since this + /// event was last fired + /// Number of outgoing packets sent since this + /// event was last fired + /// Current total number of bytes in packets we + /// are waiting on ACKs for public delegate void PacketStats(int inPackets, int outPackets, int unAckedBytes); + /// + /// Fired when the queue for a packet category is empty. This event can be + /// hooked to put more data on the empty queue + /// + /// Category of the packet queue that is empty public delegate void QueueEmpty(ThrottleOutPacketType category); - public class LLUDPClient + #endregion Delegates + + /// + /// Tracks state for a client UDP connection and provides client-specific methods + /// + public sealed class LLUDPClient { /// The number of packet categories to throttle on. If a throttle category is added /// or removed, this number must also change const int THROTTLE_CATEGORY_COUNT = 7; + /// Fired when updated networking stats are produced for this client public event PacketStats OnPacketStats; + /// Fired when the queue for a packet category is empty. This event can be + /// hooked to put more data on the empty queue public event QueueEmpty OnQueueEmpty; /// AgentID for this client @@ -115,6 +139,16 @@ namespace OpenSim.Region.ClientStack.LindenUDP /// A reference to the LLUDPServer that is managing this client private readonly LLUDPServer udpServer; + /// + /// Default constructor + /// + /// Reference to the UDP server this client is connected to + /// Default throttling rates and maximum throttle limits + /// Parent HTB (hierarchical token bucket) + /// that the child throttles will be governed by + /// Circuit code for this connection + /// AgentID for the connected agent + /// Remote endpoint for this connection 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; } + /// + /// Shuts down this client connection + /// public void Shutdown() { + // TODO: Do we need to invalidate the circuit? IsConnected = false; } + /// + /// Gets information about this client connection + /// + /// Information about the client connection 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(); info.needAck = new Dictionary(); @@ -169,8 +213,16 @@ namespace OpenSim.Region.ClientStack.LindenUDP return info; } + /// + /// Modifies the UDP throttles + /// + /// New throttling values 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 + /// + /// A shim around LLUDPServer that implements the IClientNetworkServer interface + /// + public sealed class LLUDPServerShim : IClientNetworkServer { LLUDPServer m_udpServer; @@ -80,6 +83,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP } } + /// + /// The LLUDP server for a region. This handles incoming and outgoing + /// packets for all UDP connections to the region + /// 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 { + /// + /// Holds a reference to the 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 + /// public sealed class OutgoingPacket { /// Client this packet is destined for @@ -46,6 +53,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP /// Category this packet belongs to public ThrottleOutPacketType Category; + /// + /// Default constructor + /// + /// Reference to the client this packet is destined for + /// Serialized packet data. If the flags or sequence number + /// need to be updated, they will be injected directly into this binary buffer + /// Throttling category for this packet 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 { + /// + /// 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 + /// public sealed class ThrottleRates { + /// Drip rate for resent packets public int Resend; + /// Drip rate for terrain packets public int Land; + /// Drip rate for wind packets public int Wind; + /// Drip rate for cloud packets public int Cloud; + /// Drip rate for task (state and transaction) packets public int Task; + /// Drip rate for texture packets public int Texture; + /// Drip rate for asset packets public int Asset; + /// Maximum burst rate for resent packets public int ResendLimit; + /// Maximum burst rate for land packets public int LandLimit; + /// Maximum burst rate for wind packets public int WindLimit; + /// Maximum burst rate for cloud packets public int CloudLimit; + /// Maximum burst rate for task (state and transaction) packets public int TaskLimit; + /// Maximum burst rate for texture packets public int TextureLimit; + /// Maximum burst rate for asset packets public int AssetLimit; + /// + /// Default constructor + /// + /// Config source to load defaults from 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 { + /// + /// Special collection that is optimized for tracking unacknowledged packets + /// public sealed class UnackedPacketCollection { + /// Synchronization primitive. A lock must be acquired on this + /// object before calling any of the unsafe methods public object SyncRoot = new object(); - SortedDictionary packets; + /// Holds the actual unacked packet data, sorted by sequence number + private SortedDictionary packets = new SortedDictionary(); + /// Gets the total number of unacked packets public int Count { get { return packets.Count; } } + /// + /// Default constructor + /// public UnackedPacketCollection() { - packets = new SortedDictionary(); } + /// + /// Add an unacked packet to the collection + /// + /// Packet that is awaiting acknowledgement + /// True if the packet was successfully added, false if the + /// packet already existed in the collection public bool Add(OutgoingPacket packet) { lock (SyncRoot) @@ -58,11 +73,24 @@ namespace OpenSim.Region.ClientStack.LindenUDP } } + /// + /// Removes a packet from the collection without attempting to obtain a + /// lock first + /// + /// Sequence number of the packet to remove + /// True if the packet was found and removed, otherwise false public bool RemoveUnsafe(uint sequenceNumber) { return packets.Remove(sequenceNumber); } + /// + /// Removes a packet from the collection without attempting to obtain a + /// lock first + /// + /// Sequence number of the packet to remove + /// Returns the removed packet + /// True if the packet was found and removed, otherwise false 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; } + /// + /// Gets the packet with the lowest sequence number + /// + /// The packet with the lowest sequence number, or null if the + /// collection is empty public OutgoingPacket GetOldest() { lock (SyncRoot) @@ -83,7 +116,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP } } - public List GetExpiredPackets(int timeout) + /// + /// Returns a list of all of the packets with a TickCount older than + /// the specified timeout + /// + /// Number of ticks (milliseconds) before a + /// packet is considered expired + /// A list of all expired packets according to the given + /// expiration timeout + public List GetExpiredPackets(int timeoutMS) { List 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(); diff --git a/prebuild.xml b/prebuild.xml index f863ab6..f35dcc3 100644 --- a/prebuild.xml +++ b/prebuild.xml @@ -131,6 +131,7 @@ ../../bin/ + @@ -1755,6 +1756,7 @@ ../../../../bin/ + @@ -3695,6 +3697,7 @@ + -- cgit v1.1