Prevent duplicate invocations or race dontision in SP.CompleteMovement()

This can happen under poor network conditions if a viewer repeats the message send
If this happens, physics actors can get orphaned, which unecessarily raises physics frame times
0.7.6-extended
Justin Clark-Casey (justincc) 2014-01-16 20:23:31 +00:00
parent 91d86301ef
commit bf86addf3d
2 changed files with 65 additions and 56 deletions

View File

@ -108,6 +108,16 @@ namespace OpenSim.Region.Framework.Scenes
} }
} }
/// <summary>
/// This exists to prevent race conditions between two CompleteMovement threads if the simulator is slow and
/// the viewer fires these in quick succession.
/// </summary>
/// <remarks>
/// TODO: The child -> agent transition should be folded into LifecycleState and the CompleteMovement
/// regulation done there.
/// </remarks>
private object m_completeMovementLock = new object();
// private static readonly byte[] DEFAULT_TEXTURE = AvatarAppearance.GetDefaultTexture().GetBytes(); // private static readonly byte[] DEFAULT_TEXTURE = AvatarAppearance.GetDefaultTexture().GetBytes();
private static readonly Array DIR_CONTROL_FLAGS = Enum.GetValues(typeof(Dir_ControlFlags)); private static readonly Array DIR_CONTROL_FLAGS = Enum.GetValues(typeof(Dir_ControlFlags));
private static readonly Vector3 HEAD_ADJUSTMENT = new Vector3(0f, 0f, 0.3f); private static readonly Vector3 HEAD_ADJUSTMENT = new Vector3(0f, 0f, 0.3f);
@ -904,6 +914,7 @@ namespace OpenSim.Region.Framework.Scenes
/// <summary> /// <summary>
/// Turns a child agent into a root agent. /// Turns a child agent into a root agent.
/// </summary> /// </summary>
/// <remarks>
/// Child agents are logged into neighbouring sims largely to observe changes. Root agents exist when the /// Child agents are logged into neighbouring sims largely to observe changes. Root agents exist when the
/// avatar is actual in the sim. They can perform all actions. /// avatar is actual in the sim. They can perform all actions.
/// This change is made whenever an avatar enters a region, whether by crossing over from a neighbouring sim, /// This change is made whenever an avatar enters a region, whether by crossing over from a neighbouring sim,
@ -911,8 +922,8 @@ namespace OpenSim.Region.Framework.Scenes
/// ///
/// This method is on the critical path for transferring an avatar from one region to another. Delay here /// This method is on the critical path for transferring an avatar from one region to another. Delay here
/// delays that crossing. /// delays that crossing.
/// </summary> /// </remarks>
private void MakeRootAgent(Vector3 pos, bool isFlying) private bool MakeRootAgent(Vector3 pos, bool isFlying)
{ {
// m_log.InfoFormat( // m_log.InfoFormat(
// "[SCENE]: Upgrading child to root agent for {0} in {1}", // "[SCENE]: Upgrading child to root agent for {0} in {1}",
@ -920,6 +931,10 @@ namespace OpenSim.Region.Framework.Scenes
//m_log.DebugFormat("[SCENE]: known regions in {0}: {1}", Scene.RegionInfo.RegionName, KnownChildRegionHandles.Count); //m_log.DebugFormat("[SCENE]: known regions in {0}: {1}", Scene.RegionInfo.RegionName, KnownChildRegionHandles.Count);
lock (m_completeMovementLock)
if (!IsChildAgent)
return false;
IsChildAgent = false; IsChildAgent = false;
// Must reset this here so that a teleport to a region next to an existing region does not keep the flag // Must reset this here so that a teleport to a region next to an existing region does not keep the flag
@ -1069,6 +1084,7 @@ namespace OpenSim.Region.Framework.Scenes
m_scene.EventManager.TriggerOnMakeRootAgent(this); m_scene.EventManager.TriggerOnMakeRootAgent(this);
return true;
} }
public int GetStateSource() public int GetStateSource()
@ -1442,7 +1458,14 @@ namespace OpenSim.Region.Framework.Scenes
} }
bool flying = ((m_AgentControlFlags & AgentManager.ControlFlags.AGENT_CONTROL_FLY) != 0); bool flying = ((m_AgentControlFlags & AgentManager.ControlFlags.AGENT_CONTROL_FLY) != 0);
MakeRootAgent(AbsolutePosition, flying); if (!MakeRootAgent(AbsolutePosition, flying))
{
m_log.DebugFormat(
"[SCENE PRESENCE]: Aborting CompleteMovement call for {0} in {1} as they are already root",
Name, Scene.Name);
return;
}
// Tell the client that we're totally ready // Tell the client that we're totally ready
ControllingClient.MoveAgentIntoRegion(m_scene.RegionInfo, AbsolutePosition, look); ControllingClient.MoveAgentIntoRegion(m_scene.RegionInfo, AbsolutePosition, look);

View File

@ -111,6 +111,45 @@ namespace OpenSim.Region.Framework.Scenes.Tests
Assert.That(scene.GetScenePresences().Count, Is.EqualTo(1)); Assert.That(scene.GetScenePresences().Count, Is.EqualTo(1));
} }
/// <summary>
/// Test that duplicate complete movement calls are ignored.
/// </summary>
/// <remarks>
/// If duplicate calls are not ignored then there is a risk of race conditions or other unexpected effects.
/// </remarks>
[Test]
public void TestDupeCompleteMovementCalls()
{
TestHelpers.InMethod();
// TestHelpers.EnableLogging();
UUID spUuid = TestHelpers.ParseTail(0x1);
TestScene scene = new SceneHelpers().SetupScene();
int makeRootAgentEvents = 0;
scene.EventManager.OnMakeRootAgent += spi => makeRootAgentEvents++;
ScenePresence sp = SceneHelpers.AddScenePresence(scene, spUuid);
Assert.That(makeRootAgentEvents, Is.EqualTo(1));
// Normally these would be invoked by a CompleteMovement message coming in to the UDP stack. But for
// convenience, here we will invoke it manually.
sp.CompleteMovement(sp.ControllingClient, true);
Assert.That(makeRootAgentEvents, Is.EqualTo(1));
// Check rest of exepcted parameters.
Assert.That(scene.AuthenticateHandler.GetAgentCircuitData(spUuid), Is.Not.Null);
Assert.That(scene.AuthenticateHandler.GetAgentCircuits().Count, Is.EqualTo(1));
Assert.That(sp.IsChildAgent, Is.False);
Assert.That(sp.UUID, Is.EqualTo(spUuid));
Assert.That(scene.GetScenePresences().Count, Is.EqualTo(1));
}
[Test] [Test]
public void TestCreateDuplicateRootScenePresence() public void TestCreateDuplicateRootScenePresence()
{ {
@ -249,58 +288,5 @@ namespace OpenSim.Region.Framework.Scenes.Tests
// Assert.That(childPresence, Is.Not.Null); // Assert.That(childPresence, Is.Not.Null);
// Assert.That(childPresence.IsChildAgent, Is.True); // Assert.That(childPresence.IsChildAgent, Is.True);
} }
// /// <summary>
// /// Test adding a root agent to a scene. Doesn't yet actually complete crossing the agent into the scene.
// /// </summary>
// [Test]
// public void T010_TestAddRootAgent()
// {
// TestHelpers.InMethod();
//
// string firstName = "testfirstname";
//
// AgentCircuitData agent = new AgentCircuitData();
// agent.AgentID = agent1;
// agent.firstname = firstName;
// agent.lastname = "testlastname";
// agent.SessionID = UUID.Random();
// agent.SecureSessionID = UUID.Random();
// agent.circuitcode = 123;
// agent.BaseFolder = UUID.Zero;
// agent.InventoryFolder = UUID.Zero;
// agent.startpos = Vector3.Zero;
// agent.CapsPath = GetRandomCapsObjectPath();
// agent.ChildrenCapSeeds = new Dictionary<ulong, string>();
// agent.child = true;
//
// scene.PresenceService.LoginAgent(agent.AgentID.ToString(), agent.SessionID, agent.SecureSessionID);
//
// string reason;
// scene.NewUserConnection(agent, (uint)TeleportFlags.ViaLogin, out reason);
// testclient = new TestClient(agent, scene);
// scene.AddNewAgent(testclient);
//
// ScenePresence presence = scene.GetScenePresence(agent1);
//
// Assert.That(presence, Is.Not.Null, "presence is null");
// Assert.That(presence.Firstname, Is.EqualTo(firstName), "First name not same");
// acd1 = agent;
// }
//
// /// <summary>
// /// Test removing an uncrossed root agent from a scene.
// /// </summary>
// [Test]
// public void T011_TestRemoveRootAgent()
// {
// TestHelpers.InMethod();
//
// scene.RemoveClient(agent1);
//
// ScenePresence presence = scene.GetScenePresence(agent1);
//
// Assert.That(presence, Is.Null, "presence is not null");
// }
} }
} }