From d71c6dea7e5bfe827a9d723d972a9eec4cb77826 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 8 Jun 2012 01:43:58 +0100 Subject: Store already retrieve IClientAPI in IncomingPacket structure for later use rather than doing another retrieve on dequeue. Instead of checking whether the client still exists by trying to retrieve again from the client manager, this patch gets it back from IncomingPacket and checks the IClientAPI.IsActive state. --- .../Region/ClientStack/Linden/UDP/LLUDPServer.cs | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs') diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index 09bb52c..55bda63 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -879,7 +879,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP #endregion Ping Check Handling // Inbox insertion - packetInbox.Enqueue(new IncomingPacket(udpClient, packet)); + packetInbox.Enqueue(new IncomingPacket((LLClientView)client, packet)); } #region BinaryStats @@ -1386,22 +1386,19 @@ namespace OpenSim.Region.ClientStack.LindenUDP #endregion - private void ProcessInPacket(object state) + private void ProcessInPacket(IncomingPacket incomingPacket) { - IncomingPacket incomingPacket = (IncomingPacket)state; Packet packet = incomingPacket.Packet; - LLUDPClient udpClient = incomingPacket.Client; - IClientAPI client; + LLClientView client = incomingPacket.Client; // Sanity check - if (packet == null || udpClient == null) + if (packet == null || client == null) { - m_log.WarnFormat("[LLUDPSERVER]: Processing a packet with incomplete state. Packet=\"{0}\", UDPClient=\"{1}\"", - packet, udpClient); + m_log.WarnFormat("[LLUDPSERVER]: Processing a packet with incomplete state. Packet=\"{0}\", Client=\"{1}\"", + packet, client); } - // Make sure this client is still alive - if (m_scene.TryGetClient(udpClient.AgentID, out client)) + if (client.IsActive) { m_currentIncomingClient = client; @@ -1419,8 +1416,11 @@ namespace OpenSim.Region.ClientStack.LindenUDP 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", udpClient.AgentID, packet.Type); - m_log.Error(e.Message, e); + m_log.Error( + string.Format( + "[LLUDPSERVER]: Client packet handler for {0} for packet {1} threw ", + client.Name, packet.Type), + e); } finally { @@ -1431,7 +1431,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP { m_log.DebugFormat( "[LLUDPSERVER]: Dropped incoming {0} for dead client {1} in {2}", - packet.Type, udpClient.AgentID, m_scene.RegionInfo.RegionName); + packet.Type, client.Name, m_scene.RegionInfo.RegionName); } } -- cgit v1.1 From d73805d7f488e45a00a2d25c08876d400083d27f Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 8 Jun 2012 01:51:28 +0100 Subject: Remove null checks at top of LLUDPServer.ProcessInPacket(). Neither packet nor client are ever null. --- OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs') diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index 55bda63..a292a6c 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -1391,13 +1391,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP Packet packet = incomingPacket.Packet; LLClientView client = incomingPacket.Client; - // Sanity check - if (packet == null || client == null) - { - m_log.WarnFormat("[LLUDPSERVER]: Processing a packet with incomplete state. Packet=\"{0}\", Client=\"{1}\"", - packet, client); - } - if (client.IsActive) { m_currentIncomingClient = client; @@ -1442,4 +1435,4 @@ namespace OpenSim.Region.ClientStack.LindenUDP RemoveClient(((LLClientView)client).UDPClient); } } -} +} \ No newline at end of file -- cgit v1.1 From 5f4f9f02309b7df4d1bdcc560cee96d266c48a07 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 8 Jun 2012 03:12:23 +0100 Subject: Add regression test for client logout due to ack timeout. --- OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs') diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index a292a6c..58a3b1c 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -147,11 +147,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP private int m_elapsed500MSOutgoingPacketHandler; /// Flag to signal when clients should check for resends - private bool m_resendUnacked; + protected bool m_resendUnacked; + /// Flag to signal when clients should send ACKs - private bool m_sendAcks; + protected bool m_sendAcks; + /// Flag to signal when clients should send pings - private bool m_sendPing; + protected bool m_sendPing; private int m_defaultRTO = 0; private int m_maxRTO = 0; @@ -1244,7 +1246,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP Watchdog.RemoveThread(); } - private void ClientOutgoingPacketHandler(IClientAPI client) + protected void ClientOutgoingPacketHandler(IClientAPI client) { m_currentOutgoingClient = client; -- cgit v1.1 From c215b1ad169cb8c3add70622f610e980ee9cfa31 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 8 Jun 2012 03:53:03 +0100 Subject: If logging a client out due to ack timeout, do this asynchronously rather than synchronously on the outgoing packet loop. This is the same async behaviour as normal logouts. This is necessary because the event queue will sleep the thread for 5 seconds on an ack timeout logout as the client isn't around to pick up the final event queue messages. --- .../Region/ClientStack/Linden/UDP/LLUDPServer.cs | 26 +++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs') diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index 58a3b1c..e1fccb5 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -539,8 +539,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP SendPacket(udpClient, completePing, ThrottleOutPacketType.Unknown, false, null); } - public void HandleUnacked(LLUDPClient udpClient) + public void HandleUnacked(LLClientView client) { + LLUDPClient udpClient = client.UDPClient; + if (!udpClient.IsConnected) return; @@ -553,12 +555,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP if (udpClient.IsPaused) timeoutTicks = m_pausedAckTimeout; - if ((Environment.TickCount & Int32.MaxValue) - udpClient.TickLastPacketReceived > timeoutTicks) + if (!client.IsLoggingOut && + (Environment.TickCount & Int32.MaxValue) - udpClient.TickLastPacketReceived > timeoutTicks) { m_log.Warn("[LLUDPSERVER]: Ack timeout, disconnecting " + udpClient.AgentID); StatsManager.SimExtraStats.AddAbnormalClientThreadTermination(); - RemoveClient(udpClient); + return; } @@ -1113,8 +1116,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP IClientAPI client; if (m_scene.TryGetClient(udpClient.AgentID, out client)) { + // We must set IsLoggingOut synchronously so that we can stop the packet loop reinvoking this method. client.IsLoggingOut = true; - client.Close(); + + // Fire this out on a different thread so that we don't hold up outgoing packet processing for + // everybody else if this is being called due to an ack timeout. + // This is the same as processing as the async process of a logout request. + Util.FireAndForget(o => client.Close()); } else { @@ -1254,12 +1262,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP { if (client is LLClientView) { - LLUDPClient udpClient = ((LLClientView)client).UDPClient; + LLClientView llClient = (LLClientView)client; + LLUDPClient udpClient = llClient.UDPClient; if (udpClient.IsConnected) { if (m_resendUnacked) - HandleUnacked(udpClient); + HandleUnacked(llClient); if (m_sendAcks) SendAcks(udpClient); @@ -1308,7 +1317,8 @@ namespace OpenSim.Region.ClientStack.LindenUDP { if (client is LLClientView) { - LLUDPClient udpClient = ((LLClientView)client).UDPClient; + LLClientView llClient = (LLClientView)client; + LLUDPClient udpClient = llClient.UDPClient; if (udpClient.IsConnected) { @@ -1317,7 +1327,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP nticksUnack++; watch2.Start(); - HandleUnacked(udpClient); + HandleUnacked(llClient); watch2.Stop(); avgResendUnackedTicks = (nticksUnack - 1)/(float)nticksUnack * avgResendUnackedTicks + (watch2.ElapsedTicks / (float)nticksUnack); -- cgit v1.1 From f94b92df4646162c0a8f37dd3373c6c2a20c66e3 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 8 Jun 2012 04:12:22 +0100 Subject: Instead of retrieving the known client again in LLUDPServer.RemoveClient(), check the IsLoggingOut flag instead. This is slightly better thread-race wise --- .../Region/ClientStack/Linden/UDP/LLUDPServer.cs | 31 +++++++--------------- 1 file changed, 10 insertions(+), 21 deletions(-) (limited to 'OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs') diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index e1fccb5..7f86491 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -560,7 +560,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP { m_log.Warn("[LLUDPSERVER]: Ack timeout, disconnecting " + udpClient.AgentID); StatsManager.SimExtraStats.AddAbnormalClientThreadTermination(); - RemoveClient(udpClient); + RemoveClient(client); return; } @@ -1110,26 +1110,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP return client; } - private void RemoveClient(LLUDPClient udpClient) + private void RemoveClient(IClientAPI client) { - // Remove this client from the scene - IClientAPI client; - if (m_scene.TryGetClient(udpClient.AgentID, out client)) - { - // We must set IsLoggingOut synchronously so that we can stop the packet loop reinvoking this method. - client.IsLoggingOut = true; + // We must set IsLoggingOut synchronously so that we can stop the packet loop reinvoking this method. + client.IsLoggingOut = true; - // Fire this out on a different thread so that we don't hold up outgoing packet processing for - // everybody else if this is being called due to an ack timeout. - // This is the same as processing as the async process of a logout request. - Util.FireAndForget(o => client.Close()); - } - else - { - m_log.WarnFormat( - "[LLUDPSERVER]: Tried to remove client with id {0} but not such client in {1}", - udpClient.AgentID, m_scene.RegionInfo.RegionName); - } + // Fire this out on a different thread so that we don't hold up outgoing packet processing for + // everybody else if this is being called due to an ack timeout. + // This is the same as processing as the async process of a logout request. + Util.FireAndForget(o => client.Close()); } private void IncomingPacketHandler() @@ -1443,8 +1432,8 @@ namespace OpenSim.Region.ClientStack.LindenUDP protected void LogoutHandler(IClientAPI client) { client.SendLogoutPacket(); - if (client.IsActive) - RemoveClient(((LLClientView)client).UDPClient); + if (!client.IsLoggingOut) + RemoveClient(client); } } } \ No newline at end of file -- cgit v1.1