From 55de1897527d2b464df2d0b85ce0dd612473a734 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 18:56:07 +0000 Subject: [PATCH 1/7] minor: remove some mono compiler warnings --- .../Region/Framework/Scenes/Animation/ScenePresenceAnimator.cs | 2 +- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/Animation/ScenePresenceAnimator.cs b/OpenSim/Region/Framework/Scenes/Animation/ScenePresenceAnimator.cs index 43cfd807b3..eda085f47f 100644 --- a/OpenSim/Region/Framework/Scenes/Animation/ScenePresenceAnimator.cs +++ b/OpenSim/Region/Framework/Scenes/Animation/ScenePresenceAnimator.cs @@ -60,7 +60,7 @@ namespace OpenSim.Region.Framework.Scenes.Animation public int m_animTickJump; // ScenePresence has to see this to control +Z force public bool m_jumping = false; public float m_jumpVelocity = 0f; - private int m_landing = 0; +// private int m_landing = 0; public bool Falling { get { return m_falling; } diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index a70c276393..6463ab10dd 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -2378,8 +2378,6 @@ namespace OpenSim.Region.Framework.Scenes #region Overridden Methods - private bool sendingPrims = false; - public override void Update() { const float ROTATION_TOLERANCE = 0.01f; From 0e265889dd909d23b45175ec0e6f2d52725c008c Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 19:25:24 +0000 Subject: [PATCH 2/7] Remove unnecessary AgentCircuitData null check from Scene.AddNewClient(). The only caller is the LLUDP stack and this has to validate the UDP circuit itself, so we know that it exists. This allows us to eliminate another null check elsewhere and simplifies the method contract --- OpenSim/Framework/IScene.cs | 4 +--- .../Region/ClientStack/Linden/UDP/LLUDPServer.cs | 8 ++++---- OpenSim/Region/Framework/Scenes/Scene.cs | 15 ++++----------- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 2 +- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/OpenSim/Framework/IScene.cs b/OpenSim/Framework/IScene.cs index 6919c48d91..e0e023d76c 100644 --- a/OpenSim/Framework/IScene.cs +++ b/OpenSim/Framework/IScene.cs @@ -74,9 +74,7 @@ namespace OpenSim.Framework /// /// The type of agent to add. /// - /// The scene agent if the new client was added. - /// Null if the required scene agent already existed or no scene agent was added because the required client circuit doesn't exist. - /// + /// The scene agent if the new client was added or if an agent that already existed. ISceneAgent AddNewClient(IClientAPI client, PresenceType type); /// diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index 7db5f6bc5b..4ab8fd02fd 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -897,12 +897,12 @@ namespace OpenSim.Region.ClientStack.LindenUDP IClientAPI client = AddNewClient((UseCircuitCodePacket)packet, remoteEndPoint); // Send ack straight away to let the viewer know that the connection is active. + // The client will be null if it already exists (e.g. if on a region crossing the client sends a use + // circuit code to the existing child agent. This is not particularly obvious. SendAckImmediate(remoteEndPoint, packet.Header.Sequence); - // FIXME: Nasty - this is the only way we currently know if Scene.AddNewClient() failed to find a - // circuit and bombed out early. That check might be pointless since authorization is established - // up here. - if (client != null && client.SceneAgent != null) + // We only want to send initial data to new clients, not ones which are being converted from child to root. + if (client != null) client.SceneAgent.SendInitialDataToMe(); // m_log.DebugFormat( diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index d47536a866..11505cc040 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -2486,21 +2486,14 @@ namespace OpenSim.Region.Framework.Scenes #region Add/Remove Avatar Methods - /// - /// Add a new client and create a child scene presence for it. - /// - /// - /// The type of agent to add. public override ISceneAgent AddNewClient(IClientAPI client, PresenceType type) { + // Validation occurs in LLUDPServer AgentCircuitData aCircuit = m_authenticateHandler.GetAgentCircuitData(client.CircuitCode); - bool vialogin = false; - if (aCircuit == null) // no good, didn't pass NewUserConnection successfully - return null; - - vialogin = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0 || - (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0; + bool vialogin + = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0 + || (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0; CheckHeartbeat(); diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 6463ab10dd..882492178e 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -2548,7 +2548,7 @@ namespace OpenSim.Region.Framework.Scenes } // This agent just became root. We are going to tell everyone about it. The process of - // getting other avatars information was initiated in the constructor... don't do it + // getting other avatars information was initiated elsewhere immediately after the child circuit connected... don't do it // again here... this comes after the cached appearance check because the avatars // appearance goes into the avatar update packet SendAvatarDataToAllAgents(); From bc13855e6475ce2df051ad20acdd76756fb22555 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 20:52:34 +0000 Subject: [PATCH 3/7] Reactivate BasicCircuitTests.TestAddClient() This checks that the initial UseCircuitCode packet is handled correctly for a normal client login. --- OpenSim/Framework/Util.cs | 45 ++-- .../ClientStack/Linden/UDP/LLUDPServer.cs | 20 +- .../ClientStack/Linden/UDP/OpenSimUDPBase.cs | 2 +- .../Linden/UDP/Tests/BasicCircuitTests.cs | 225 +++++++++--------- ...ion.ClientStack.LindenUDP.Tests.dll.config | 33 +++ 5 files changed, 200 insertions(+), 125 deletions(-) create mode 100644 bin/OpenSim.Region.ClientStack.LindenUDP.Tests.dll.config diff --git a/OpenSim/Framework/Util.cs b/OpenSim/Framework/Util.cs index 21cfc09555..ed92b2d2f4 100644 --- a/OpenSim/Framework/Util.cs +++ b/OpenSim/Framework/Util.cs @@ -58,10 +58,12 @@ namespace OpenSim.Framework /// /// None is used to execute the method in the same thread that made the call. It should only be used by regression /// test code that relies on predictable event ordering. + /// RegressionTest is used by regression tests. It fires the call synchronously and does not catch any exceptions. /// public enum FireAndForgetMethod { None, + RegressionTest, UnsafeQueueUserWorkItem, QueueUserWorkItem, BeginInvoke, @@ -1544,27 +1546,38 @@ namespace OpenSim.Framework public static void FireAndForget(System.Threading.WaitCallback callback, object obj) { - // When OpenSim interacts with a database or sends data over the wire, it must send this in en_US culture - // so that we don't encounter problems where, for instance, data is saved with a culture that uses commas - // for decimals places but is read by a culture that treats commas as number seperators. - WaitCallback realCallback = delegate(object o) - { - Culture.SetCurrentCulture(); + WaitCallback realCallback; - try + if (FireAndForgetMethod == FireAndForgetMethod.RegressionTest) + { + // If we're running regression tests, then we want any exceptions to rise up to the test code. + realCallback = o => { Culture.SetCurrentCulture(); callback(o); }; + } + else + { + // When OpenSim interacts with a database or sends data over the wire, it must send this in en_US culture + // so that we don't encounter problems where, for instance, data is saved with a culture that uses commas + // for decimals places but is read by a culture that treats commas as number seperators. + realCallback = o => { - callback(o); - } - catch (Exception e) - { - m_log.ErrorFormat( - "[UTIL]: Continuing after async_call_method thread terminated with exception {0}{1}", - e.Message, e.StackTrace); - } - }; + Culture.SetCurrentCulture(); + + try + { + callback(o); + } + catch (Exception e) + { + m_log.ErrorFormat( + "[UTIL]: Continuing after async_call_method thread terminated with exception {0}{1}", + e.Message, e.StackTrace); + } + }; + } switch (FireAndForgetMethod) { + case FireAndForgetMethod.RegressionTest: case FireAndForgetMethod.None: realCallback.Invoke(obj); break; diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index 4ab8fd02fd..ac21805c54 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -611,11 +611,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP outgoingPacket.TickCount = Environment.TickCount & Int32.MaxValue; } - protected override void PacketReceived(UDPPacketBuffer buffer) + public override void PacketReceived(UDPPacketBuffer buffer) { // Debugging/Profiling //try { Thread.CurrentThread.Name = "PacketReceived (" + m_scene.RegionInfo.RegionName + ")"; } //catch (Exception) { } +// m_log.DebugFormat( +// "[LLUDPSERVER]: Packet received from {0} in {1}", buffer.RemoteEndPoint, m_scene.RegionInfo.RegionName); LLUDPClient udpClient = null; Packet packet = null; @@ -625,7 +627,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP #region Decoding if (buffer.DataLength < 7) + { +// m_log.WarnFormat( +// "[LLUDPSERVER]: Dropping undersized packet with {0} bytes received from {1} in {2}", +// buffer.DataLength, buffer.RemoteEndPoint, m_scene.RegionInfo.RegionName); + return; // Drop undersizd packet + } int headerLen = 7; if (buffer.Data[6] == 0xFF) @@ -637,7 +645,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP } if (buffer.DataLength < headerLen) + { +// m_log.WarnFormat( +// "[LLUDPSERVER]: Dropping packet with malformed header received from {0} in {1}", +// buffer.RemoteEndPoint, m_scene.RegionInfo.RegionName); + return; // Malformed header + } try { @@ -650,6 +664,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP } catch (IndexOutOfRangeException) { +// m_log.WarnFormat( +// "[LLUDPSERVER]: Dropping short packet received from {0} in {1}", +// buffer.RemoteEndPoint, m_scene.RegionInfo.RegionName); + return; // Drop short packet } catch(Exception e) diff --git a/OpenSim/Region/ClientStack/Linden/UDP/OpenSimUDPBase.cs b/OpenSim/Region/ClientStack/Linden/UDP/OpenSimUDPBase.cs index 6eebd9df3a..039379d4ee 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/OpenSimUDPBase.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/OpenSimUDPBase.cs @@ -44,7 +44,7 @@ namespace OpenMetaverse /// This method is called when an incoming packet is received /// /// Incoming packet buffer - protected abstract void PacketReceived(UDPPacketBuffer buffer); + public abstract void PacketReceived(UDPPacketBuffer buffer); /// UDP port to bind to in server mode protected int m_udpPort; diff --git a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs index 9d37cdfc3a..e3b0eab121 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs @@ -25,6 +25,7 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +using System; using System.Net; using log4net.Config; using Nini.Config; @@ -32,10 +33,11 @@ using NUnit.Framework; using OpenMetaverse; using OpenMetaverse.Packets; using OpenSim.Framework; +using OpenSim.Region.Framework.Scenes; using OpenSim.Tests.Common; using OpenSim.Tests.Common.Mock; -namespace OpenSim.Region.ClientStack.LindenUDP.Tests +namespace OpenSim.Region.ClientStack.LindenUDP { /// /// This will contain basic tests for the LindenUDP client stack @@ -43,19 +45,22 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests [TestFixture] public class BasicCircuitTests { - [SetUp] - public void Init() + [TestFixtureSetUp] + public void FixtureInit() { - try - { - XmlConfigurator.Configure(); - } - catch - { - // I don't care, just leave log4net off - } + // Don't allow tests to be bamboozled by asynchronous events. Execute everything on the same thread. + Util.FireAndForgetMethod = FireAndForgetMethod.RegressionTest; } - + + [TestFixtureTearDown] + public void TearDown() + { + // We must set this back afterwards, otherwise later tests will fail since they're expecting multiple + // threads. Possibly, later tests should be rewritten so none of them require async stuff (which regression + // tests really shouldn't). + Util.FireAndForgetMethod = Util.DefaultFireAndForgetMethod; + } + // /// // /// Add a client for testing // /// @@ -78,54 +83,54 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests // testLLUDPServer.LocalScene = scene; // } - /// - /// Set up a client for tests which aren't concerned with this process itself and where only one client is being - /// tested - /// - /// - /// - /// - /// - protected void AddClient( - uint circuitCode, EndPoint epSender, TestLLUDPServer testLLUDPServer, AgentCircuitManager acm) - { - UUID myAgentUuid = UUID.Parse("00000000-0000-0000-0000-000000000001"); - UUID mySessionUuid = UUID.Parse("00000000-0000-0000-0000-000000000002"); - - AddClient(circuitCode, epSender, myAgentUuid, mySessionUuid, testLLUDPServer, acm); - } +// /// +// /// Set up a client for tests which aren't concerned with this process itself and where only one client is being +// /// tested +// /// +// /// +// /// +// /// +// /// +// protected void AddClient( +// uint circuitCode, EndPoint epSender, TestLLUDPServer testLLUDPServer, AgentCircuitManager acm) +// { +// UUID myAgentUuid = UUID.Parse("00000000-0000-0000-0000-000000000001"); +// UUID mySessionUuid = UUID.Parse("00000000-0000-0000-0000-000000000002"); +// +// AddClient(circuitCode, epSender, myAgentUuid, mySessionUuid, testLLUDPServer, acm); +// } - /// - /// Set up a client for tests which aren't concerned with this process itself - /// - /// - /// - /// - /// - /// - /// - protected void AddClient( - uint circuitCode, EndPoint epSender, UUID agentId, UUID sessionId, - TestLLUDPServer testLLUDPServer, AgentCircuitManager acm) - { - AgentCircuitData acd = new AgentCircuitData(); - acd.AgentID = agentId; - acd.SessionID = sessionId; - - UseCircuitCodePacket uccp = new UseCircuitCodePacket(); - - UseCircuitCodePacket.CircuitCodeBlock uccpCcBlock - = new UseCircuitCodePacket.CircuitCodeBlock(); - uccpCcBlock.Code = circuitCode; - uccpCcBlock.ID = agentId; - uccpCcBlock.SessionID = sessionId; - uccp.CircuitCode = uccpCcBlock; - - acm.AddNewCircuit(circuitCode, acd); - - testLLUDPServer.LoadReceive(uccp, epSender); - testLLUDPServer.ReceiveData(null); - } +// /// +// /// Set up a client for tests which aren't concerned with this process itself +// /// +// /// +// /// +// /// +// /// +// /// +// /// +// protected void AddClient( +// uint circuitCode, EndPoint epSender, UUID agentId, UUID sessionId, +// TestLLUDPServer testLLUDPServer, AgentCircuitManager acm) +// { +// AgentCircuitData acd = new AgentCircuitData(); +// acd.AgentID = agentId; +// acd.SessionID = sessionId; +// +// UseCircuitCodePacket uccp = new UseCircuitCodePacket(); +// +// UseCircuitCodePacket.CircuitCodeBlock uccpCcBlock +// = new UseCircuitCodePacket.CircuitCodeBlock(); +// uccpCcBlock.Code = circuitCode; +// uccpCcBlock.ID = agentId; +// uccpCcBlock.SessionID = sessionId; +// uccp.CircuitCode = uccpCcBlock; +// +// acm.AddNewCircuit(circuitCode, acd); +// +// testLLUDPServer.LoadReceive(uccp, epSender); +// testLLUDPServer.ReceiveData(null); +// } /// /// Build an object name packet for test purposes @@ -144,54 +149,60 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests return onp; } -// /// -// /// Test adding a client to the stack -// /// -// [Test] -// public void TestAddClient() -// { -// TestHelper.InMethod(); -// -// uint myCircuitCode = 123456; -// UUID myAgentUuid = UUID.Parse("00000000-0000-0000-0000-000000000001"); -// UUID mySessionUuid = UUID.Parse("00000000-0000-0000-0000-000000000002"); -// -// TestLLUDPServer testLLUDPServer; -// TestLLPacketServer testLLPacketServer; -// AgentCircuitManager acm; -// SetupStack(new MockScene(), out testLLUDPServer, out testLLPacketServer, out acm); -// -// AgentCircuitData acd = new AgentCircuitData(); -// acd.AgentID = myAgentUuid; -// acd.SessionID = mySessionUuid; -// -// UseCircuitCodePacket uccp = new UseCircuitCodePacket(); -// -// UseCircuitCodePacket.CircuitCodeBlock uccpCcBlock -// = new UseCircuitCodePacket.CircuitCodeBlock(); -// uccpCcBlock.Code = myCircuitCode; -// uccpCcBlock.ID = myAgentUuid; -// uccpCcBlock.SessionID = mySessionUuid; -// uccp.CircuitCode = uccpCcBlock; -// -// EndPoint testEp = new IPEndPoint(IPAddress.Loopback, 999); -// -// testLLUDPServer.LoadReceive(uccp, testEp); -// testLLUDPServer.ReceiveData(null); -// -// // Circuit shouildn't exist since the circuit manager doesn't know about this circuit for authentication yet -// Assert.IsFalse(testLLUDPServer.HasCircuit(myCircuitCode)); -// -// acm.AddNewCircuit(myCircuitCode, acd); -// -// testLLUDPServer.LoadReceive(uccp, testEp); -// testLLUDPServer.ReceiveData(null); -// -// // Should succeed now -// Assert.IsTrue(testLLUDPServer.HasCircuit(myCircuitCode)); -// Assert.IsFalse(testLLUDPServer.HasCircuit(101)); -// } -// + /// + /// Test adding a client to the stack + /// + [Test] + public void TestAddClient() + { + TestHelpers.InMethod(); + XmlConfigurator.Configure(); + + TestScene scene = SceneHelpers.SetupScene(); + uint myCircuitCode = 123456; + UUID myAgentUuid = TestHelpers.ParseTail(0x1); + UUID mySessionUuid = TestHelpers.ParseTail(0x2); + IPEndPoint testEp = new IPEndPoint(IPAddress.Loopback, 999); + + uint port = 0; + AgentCircuitManager acm = scene.AuthenticateHandler; + + LLUDPServer llUdpServer + = new LLUDPServer(IPAddress.Any, ref port, 0, false, new IniConfigSource(), acm); + llUdpServer.AddScene(scene); + + UseCircuitCodePacket uccp = new UseCircuitCodePacket(); + + UseCircuitCodePacket.CircuitCodeBlock uccpCcBlock + = new UseCircuitCodePacket.CircuitCodeBlock(); + uccpCcBlock.Code = myCircuitCode; + uccpCcBlock.ID = myAgentUuid; + uccpCcBlock.SessionID = mySessionUuid; + uccp.CircuitCode = uccpCcBlock; + + byte[] uccpBytes = uccp.ToBytes(); + UDPPacketBuffer upb = new UDPPacketBuffer(testEp, uccpBytes.Length); + upb.DataLength = uccpBytes.Length; // God knows why this isn't set by the constructor. + Buffer.BlockCopy(uccpBytes, 0, upb.Data, 0, uccpBytes.Length); + + llUdpServer.PacketReceived(upb); + + // Presence shouldn't exist since the circuit manager doesn't know about this circuit for authentication yet + Assert.That(scene.GetScenePresence(myAgentUuid), Is.Null); + + AgentCircuitData acd = new AgentCircuitData(); + acd.AgentID = myAgentUuid; + acd.SessionID = mySessionUuid; + + acm.AddNewCircuit(myCircuitCode, acd); + + llUdpServer.PacketReceived(upb); + + // Should succeed now + ScenePresence sp = scene.GetScenePresence(myAgentUuid); + Assert.That(sp.UUID, Is.EqualTo(myAgentUuid)); + } + // /// // /// Test removing a client from the stack // /// diff --git a/bin/OpenSim.Region.ClientStack.LindenUDP.Tests.dll.config b/bin/OpenSim.Region.ClientStack.LindenUDP.Tests.dll.config new file mode 100644 index 0000000000..a3f681d89e --- /dev/null +++ b/bin/OpenSim.Region.ClientStack.LindenUDP.Tests.dll.config @@ -0,0 +1,33 @@ + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 14e407aff304a176bb5bd8bbca061ad0960f3fc1 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 20:55:38 +0000 Subject: [PATCH 4/7] Add OpenSim.Region.ClientStack.LindenUDP.Tests.dll back into the test suite --- .nant/local.include | 4 +--- .../Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.nant/local.include b/.nant/local.include index 94f510f96f..a9ba17d8e8 100644 --- a/.nant/local.include +++ b/.nant/local.include @@ -117,12 +117,10 @@ - @@ -351,7 +349,7 @@ - + diff --git a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs index e3b0eab121..c6a5b9859c 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs @@ -156,7 +156,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP public void TestAddClient() { TestHelpers.InMethod(); - XmlConfigurator.Configure(); +// XmlConfigurator.Configure(); TestScene scene = SceneHelpers.SetupScene(); uint myCircuitCode = 123456; From 32d0ef89c6fbaf14e669e9dd3a8508bc817fbd52 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 21:45:02 +0000 Subject: [PATCH 5/7] Extend TestAddClient() to check that the first packet received is an ack packet --- .../ClientStack/Linden/UDP/LLUDPServer.cs | 16 +- .../Linden/UDP/Tests/BasicCircuitTests.cs | 16 +- .../Linden/UDP/Tests/TestLLUDPServer.cs | 181 +++++++++--------- 3 files changed, 118 insertions(+), 95 deletions(-) diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index ac21805c54..cef5f74257 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -328,7 +328,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP /// The method to call if the packet is not acked by the client. If null, then a standard /// resend of the packet is done. /// - public void SendPacket( + public virtual void SendPacket( LLUDPClient udpClient, Packet packet, ThrottleOutPacketType category, bool allowSplitting, UnackedPacketMethod method) { // CoarseLocationUpdate packets cannot be split in an automated way @@ -928,6 +928,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP // buffer.RemoteEndPoint, (DateTime.Now - startTime).Milliseconds); } + /// + /// Send an ack immediately to the given endpoint. + /// + /// + /// FIXME: Might be possible to use SendPacketData() like everything else, but this will require refactoring so + /// that we can obtain the UDPClient easily at this point. + /// + /// + /// private void SendAckImmediate(IPEndPoint remoteEndpoint, uint sequenceNumber) { PacketAckPacket ack = new PacketAckPacket(); @@ -936,6 +945,11 @@ namespace OpenSim.Region.ClientStack.LindenUDP ack.Packets[0] = new PacketAckPacket.PacketsBlock(); ack.Packets[0].ID = sequenceNumber; + SendAckImmediate(remoteEndpoint, ack); + } + + public virtual void SendAckImmediate(IPEndPoint remoteEndpoint, PacketAckPacket ack) + { byte[] packetData = ack.ToBytes(); int length = packetData.Length; diff --git a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs index c6a5b9859c..1457194eed 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs @@ -37,7 +37,7 @@ using OpenSim.Region.Framework.Scenes; using OpenSim.Tests.Common; using OpenSim.Tests.Common.Mock; -namespace OpenSim.Region.ClientStack.LindenUDP +namespace OpenSim.Region.ClientStack.LindenUDP.Tests { /// /// This will contain basic tests for the LindenUDP client stack @@ -167,8 +167,8 @@ namespace OpenSim.Region.ClientStack.LindenUDP uint port = 0; AgentCircuitManager acm = scene.AuthenticateHandler; - LLUDPServer llUdpServer - = new LLUDPServer(IPAddress.Any, ref port, 0, false, new IniConfigSource(), acm); + TestLLUDPServer llUdpServer + = new TestLLUDPServer(IPAddress.Any, ref port, 0, false, new IniConfigSource(), acm); llUdpServer.AddScene(scene); UseCircuitCodePacket uccp = new UseCircuitCodePacket(); @@ -201,6 +201,16 @@ namespace OpenSim.Region.ClientStack.LindenUDP // Should succeed now ScenePresence sp = scene.GetScenePresence(myAgentUuid); Assert.That(sp.UUID, Is.EqualTo(myAgentUuid)); + + // FIXME: We're still replying to an ack when the client is not authorized, which is not correct behaviour. + Assert.That(llUdpServer.PacketsSent.Count, Is.EqualTo(2)); + + Packet packet = llUdpServer.PacketsSent[1]; + Assert.That(packet, Is.InstanceOf(typeof(PacketAckPacket))); + + PacketAckPacket ackPacket = packet as PacketAckPacket; + Assert.That(ackPacket.Packets.Length, Is.EqualTo(1)); + Assert.That(ackPacket.Packets[0].ID, Is.EqualTo(0)); } // /// diff --git a/OpenSim/Region/ClientStack/Linden/UDP/Tests/TestLLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/Tests/TestLLUDPServer.cs index dd7999aa91..0302385ad2 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/Tests/TestLLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/Tests/TestLLUDPServer.cs @@ -36,107 +36,106 @@ using OpenSim.Framework; namespace OpenSim.Region.ClientStack.LindenUDP.Tests { /// - /// This class enables synchronous testing of the LLUDPServer by allowing us to load our own data into the end - /// receive event + /// This class enables regression testing of the LLUDPServer by allowing us to intercept outgoing data. /// public class TestLLUDPServer : LLUDPServer { + public List PacketsSent { get; private set; } + public TestLLUDPServer(IPAddress listenIP, ref uint port, int proxyPortOffsetParm, bool allow_alternate_port, IConfigSource configSource, AgentCircuitManager circuitManager) : base(listenIP, ref port, proxyPortOffsetParm, allow_alternate_port, configSource, circuitManager) - {} + { + PacketsSent = new List(); + } - /// - /// The chunks of data to pass to the LLUDPServer when it calls EndReceive - /// - protected Queue m_chunksToLoad = new Queue(); - -// protected override void BeginReceive() -// { -// if (m_chunksToLoad.Count > 0 && m_chunksToLoad.Peek().BeginReceiveException) -// { -// ChunkSenderTuple tuple = m_chunksToLoad.Dequeue(); -// reusedEpSender = tuple.Sender; -// throw new SocketException(); -// } -// } - -// protected override bool EndReceive(out int numBytes, IAsyncResult result, ref EndPoint epSender) -// { -// numBytes = 0; -// -// //m_log.Debug("Queue size " + m_chunksToLoad.Count); -// -// if (m_chunksToLoad.Count <= 0) -// return false; -// -// ChunkSenderTuple tuple = m_chunksToLoad.Dequeue(); -// RecvBuffer = tuple.Data; -// numBytes = tuple.Data.Length; -// epSender = tuple.Sender; -// -// return true; -// } - -// public override void SendPacketTo(byte[] buffer, int size, SocketFlags flags, uint circuitcode) -// { -// // Don't do anything just yet -// } - - /// - /// Signal that this chunk should throw an exception on Socket.BeginReceive() - /// - /// - public void LoadReceiveWithBeginException(EndPoint epSender) + public override void SendAckImmediate(IPEndPoint remoteEndpoint, PacketAckPacket ack) { - ChunkSenderTuple tuple = new ChunkSenderTuple(epSender); - tuple.BeginReceiveException = true; - m_chunksToLoad.Enqueue(tuple); + PacketsSent.Add(ack); } - - /// - /// Load some data to be received by the LLUDPServer on the next receive call - /// - /// - /// - public void LoadReceive(byte[] data, EndPoint epSender) - { - m_chunksToLoad.Enqueue(new ChunkSenderTuple(data, epSender)); - } - - /// - /// Load a packet to be received by the LLUDPServer on the next receive call - /// - /// - public void LoadReceive(Packet packet, EndPoint epSender) - { - LoadReceive(packet.ToBytes(), epSender); - } - - /// - /// Calls the protected asynchronous result method. This fires out all data chunks currently queued for send - /// - /// - public void ReceiveData(IAsyncResult result) - { - // Doesn't work the same way anymore -// while (m_chunksToLoad.Count > 0) -// OnReceivedData(result); - } - - /// - /// Has a circuit with the given code been established? - /// - /// - /// - public bool HasCircuit(uint circuitCode) - { -// lock (clientCircuits_reverse) -// { -// return clientCircuits_reverse.ContainsKey(circuitCode); -// } - return true; + public override void SendPacket( + LLUDPClient udpClient, Packet packet, ThrottleOutPacketType category, bool allowSplitting, UnackedPacketMethod method) + { + PacketsSent.Add(packet); } + +//// /// +//// /// The chunks of data to pass to the LLUDPServer when it calls EndReceive +//// /// +//// protected Queue m_chunksToLoad = new Queue(); +// +//// protected override void BeginReceive() +//// { +//// if (m_chunksToLoad.Count > 0 && m_chunksToLoad.Peek().BeginReceiveException) +//// { +//// ChunkSenderTuple tuple = m_chunksToLoad.Dequeue(); +//// reusedEpSender = tuple.Sender; +//// throw new SocketException(); +//// } +//// } +// +//// protected override bool EndReceive(out int numBytes, IAsyncResult result, ref EndPoint epSender) +//// { +//// numBytes = 0; +//// +//// //m_log.Debug("Queue size " + m_chunksToLoad.Count); +//// +//// if (m_chunksToLoad.Count <= 0) +//// return false; +//// +//// ChunkSenderTuple tuple = m_chunksToLoad.Dequeue(); +//// RecvBuffer = tuple.Data; +//// numBytes = tuple.Data.Length; +//// epSender = tuple.Sender; +//// +//// return true; +//// } +// +//// public override void SendPacketTo(byte[] buffer, int size, SocketFlags flags, uint circuitcode) +//// { +//// // Don't do anything just yet +//// } +// +// /// +// /// Signal that this chunk should throw an exception on Socket.BeginReceive() +// /// +// /// +// public void LoadReceiveWithBeginException(EndPoint epSender) +// { +// ChunkSenderTuple tuple = new ChunkSenderTuple(epSender); +// tuple.BeginReceiveException = true; +// m_chunksToLoad.Enqueue(tuple); +// } +// +// /// +// /// Load some data to be received by the LLUDPServer on the next receive call +// /// +// /// +// /// +// public void LoadReceive(byte[] data, EndPoint epSender) +// { +// m_chunksToLoad.Enqueue(new ChunkSenderTuple(data, epSender)); +// } +// +// /// +// /// Load a packet to be received by the LLUDPServer on the next receive call +// /// +// /// +// public void LoadReceive(Packet packet, EndPoint epSender) +// { +// LoadReceive(packet.ToBytes(), epSender); +// } +// +// /// +// /// Calls the protected asynchronous result method. This fires out all data chunks currently queued for send +// /// +// /// +// public void ReceiveData(IAsyncResult result) +// { +// // Doesn't work the same way anymore +//// while (m_chunksToLoad.Count > 0) +//// OnReceivedData(result); +// } } /// From 50eebb5cba3bc4143115fca0e163ebdf3fc4dc60 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 22:00:59 +0000 Subject: [PATCH 6/7] Don't reply with an ack packet if the client is not authorized. --- .../ClientStack/Linden/UDP/LLUDPServer.cs | 71 ++++++++----------- .../Linden/UDP/Tests/BasicCircuitTests.cs | 5 +- 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs index cef5f74257..5610c099eb 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs @@ -905,23 +905,40 @@ namespace OpenSim.Region.ClientStack.LindenUDP // DateTime startTime = DateTime.Now; object[] array = (object[])o; UDPPacketBuffer buffer = (UDPPacketBuffer)array[0]; - UseCircuitCodePacket packet = (UseCircuitCodePacket)array[1]; + UseCircuitCodePacket uccp = (UseCircuitCodePacket)array[1]; m_log.DebugFormat("[LLUDPSERVER]: Handling UseCircuitCode request from {0}", buffer.RemoteEndPoint); IPEndPoint remoteEndPoint = (IPEndPoint)buffer.RemoteEndPoint; - // Begin the process of adding the client to the simulator - IClientAPI client = AddNewClient((UseCircuitCodePacket)packet, remoteEndPoint); - - // Send ack straight away to let the viewer know that the connection is active. - // The client will be null if it already exists (e.g. if on a region crossing the client sends a use - // circuit code to the existing child agent. This is not particularly obvious. - SendAckImmediate(remoteEndPoint, packet.Header.Sequence); - - // We only want to send initial data to new clients, not ones which are being converted from child to root. - if (client != null) - client.SceneAgent.SendInitialDataToMe(); + AuthenticateResponse sessionInfo; + if (IsClientAuthorized(uccp, out sessionInfo)) + { + // Begin the process of adding the client to the simulator + IClientAPI client + = AddClient( + uccp.CircuitCode.Code, + uccp.CircuitCode.ID, + uccp.CircuitCode.SessionID, + remoteEndPoint, + sessionInfo); + + // Send ack straight away to let the viewer know that the connection is active. + // The client will be null if it already exists (e.g. if on a region crossing the client sends a use + // circuit code to the existing child agent. This is not particularly obvious. + SendAckImmediate(remoteEndPoint, uccp.Header.Sequence); + + // We only want to send initial data to new clients, not ones which are being converted from child to root. + if (client != null) + client.SceneAgent.SendInitialDataToMe(); + } + else + { + // Don't create clients for unauthorized requesters. + m_log.WarnFormat( + "[LLUDPSERVER]: Connection request for client {0} connecting with unnotified circuit code {1} from {2}", + uccp.CircuitCode.ID, uccp.CircuitCode.Code, remoteEndPoint); + } // m_log.DebugFormat( // "[LLUDPSERVER]: Handling UseCircuitCode request from {0} took {1}ms", @@ -971,36 +988,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP return sessionInfo.Authorised; } - /// - /// Add a new client. - /// - /// - /// - /// - /// The client that was added or null if the client failed authorization or already existed. - /// - private IClientAPI AddNewClient(UseCircuitCodePacket useCircuitCode, IPEndPoint remoteEndPoint) - { - UUID agentID = useCircuitCode.CircuitCode.ID; - UUID sessionID = useCircuitCode.CircuitCode.SessionID; - uint circuitCode = useCircuitCode.CircuitCode.Code; - - AuthenticateResponse sessionInfo; - if (IsClientAuthorized(useCircuitCode, out sessionInfo)) - { - return AddClient(circuitCode, agentID, sessionID, remoteEndPoint, sessionInfo); - } - else - { - // Don't create circuits for unauthorized clients - m_log.WarnFormat( - "[LLUDPSERVER]: Connection request for client {0} connecting with unnotified circuit code {1} from {2}", - useCircuitCode.CircuitCode.ID, useCircuitCode.CircuitCode.Code, remoteEndPoint); - - return null; - } - } - /// /// Add a client. /// diff --git a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs index 1457194eed..a575e3616d 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/Tests/BasicCircuitTests.cs @@ -202,10 +202,9 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests ScenePresence sp = scene.GetScenePresence(myAgentUuid); Assert.That(sp.UUID, Is.EqualTo(myAgentUuid)); - // FIXME: We're still replying to an ack when the client is not authorized, which is not correct behaviour. - Assert.That(llUdpServer.PacketsSent.Count, Is.EqualTo(2)); + Assert.That(llUdpServer.PacketsSent.Count, Is.EqualTo(1)); - Packet packet = llUdpServer.PacketsSent[1]; + Packet packet = llUdpServer.PacketsSent[0]; Assert.That(packet, Is.InstanceOf(typeof(PacketAckPacket))); PacketAckPacket ackPacket = packet as PacketAckPacket; From 63fe673af1974e265c5d18dc95f963fd968afd26 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 8 Dec 2011 23:45:53 +0000 Subject: [PATCH 7/7] Revert "Revert "Stop performing the asset save part of baked texture uploading on the UploadBakedTexture cap asynchronously."" This turned out not to be the upload texture issue. This reverts commit 8721841fc3944ce0cdf5ce76297e73f9ed269751. --- .../UploadBakedTextureHandler.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/OpenSim/Capabilities/Handlers/UploadBakedTexture/UploadBakedTextureHandler.cs b/OpenSim/Capabilities/Handlers/UploadBakedTexture/UploadBakedTextureHandler.cs index b7ca7030bc..c637ccf180 100644 --- a/OpenSim/Capabilities/Handlers/UploadBakedTexture/UploadBakedTextureHandler.cs +++ b/OpenSim/Capabilities/Handlers/UploadBakedTexture/UploadBakedTextureHandler.cs @@ -104,7 +104,7 @@ namespace OpenSim.Capabilities.Handlers } catch (Exception e) { - m_log.Error("[CAPS]: " + e.ToString()); + m_log.Error("[UPLOAD BAKED TEXTURE HANDLER]: " + e.ToString()); } return null; @@ -130,6 +130,8 @@ namespace OpenSim.Capabilities.Handlers class BakedTextureUploader { +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); + public event Action OnUpLoad; private string uploaderPath = String.Empty; @@ -154,10 +156,12 @@ namespace OpenSim.Capabilities.Handlers public string uploaderCaps(byte[] data, string path, string param) { Action handlerUpLoad = OnUpLoad; + + // Don't do this asynchronously, otherwise it's possible for the client to send set appearance information + // on another thread which might send out avatar updates before the asset has been put into the asset + // service. if (handlerUpLoad != null) - { - Util.FireAndForget(delegate(object o) { handlerUpLoad(newAssetID, data); }); - } + handlerUpLoad(newAssetID, data); string res = String.Empty; LLSDAssetUploadComplete uploadComplete = new LLSDAssetUploadComplete(); @@ -169,7 +173,7 @@ namespace OpenSim.Capabilities.Handlers httpListener.RemoveStreamHandler("POST", uploaderPath); - // m_log.InfoFormat("[CAPS] baked texture upload completed for {0}",newAssetID); +// m_log.DebugFormat("[BAKED TEXTURE UPLOADER]: baked texture upload completed for {0}", newAssetID); return res; }