Don't make duplicate call to ScenePresence.Close() separately in ETM.DoTeleport() if an agent needs closing.

This is always done as part of Scene.RemoveClient()
Also refactors try/catching in Scene.RemoveClient() to log NREs instead of silently discarding, since these are useful symptoms of problems.
0.7.3-extended
Justin Clark-Casey (justincc) 2012-06-08 01:24:44 +01:00
parent ba0ebe6d75
commit 498154af80
4 changed files with 77 additions and 85 deletions

View File

@ -638,7 +638,6 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
// an agent cannot teleport back to this region if it has teleported away. // an agent cannot teleport back to this region if it has teleported away.
Thread.Sleep(2000); Thread.Sleep(2000);
sp.Close();
sp.Scene.IncomingCloseAgent(sp.UUID); sp.Scene.IncomingCloseAgent(sp.UUID);
} }
else else

View File

@ -3211,10 +3211,23 @@ namespace OpenSim.Region.Framework.Scenes
// CheckHeartbeat(); // CheckHeartbeat();
bool isChildAgent = false; bool isChildAgent = false;
ScenePresence avatar = GetScenePresence(agentID); ScenePresence avatar = GetScenePresence(agentID);
if (avatar != null)
if (avatar == null)
{
m_log.WarnFormat(
"[SCENE]: Called RemoveClient() with agent ID {0} but no such presence is in the scene.", agentID);
return;
}
try
{ {
isChildAgent = avatar.IsChildAgent; isChildAgent = avatar.IsChildAgent;
m_log.DebugFormat(
"[SCENE]: Removing {0} agent {1} {2} from {3}",
(isChildAgent ? "child" : "root"), avatar.Name, agentID, RegionInfo.RegionName);
// Don't do this to root agents, it's not nice for the viewer // Don't do this to root agents, it's not nice for the viewer
if (closeChildAgents && isChildAgent) if (closeChildAgents && isChildAgent)
{ {
@ -3236,99 +3249,77 @@ namespace OpenSim.Region.Framework.Scenes
avatar.StandUp(); avatar.StandUp();
} }
try m_sceneGraph.removeUserCount(!isChildAgent);
// TODO: We shouldn't use closeChildAgents here - it's being used by the NPC module to stop
// unnecessary operations. This should go away once NPCs have no accompanying IClientAPI
if (closeChildAgents && CapsModule != null)
CapsModule.RemoveCaps(agentID);
// REFACTORING PROBLEM -- well not really a problem, but just to point out that whatever
// this method is doing is HORRIBLE!!!
avatar.Scene.NeedSceneCacheClear(avatar.UUID);
if (closeChildAgents && !isChildAgent)
{ {
m_log.DebugFormat( List<ulong> regions = avatar.KnownRegionHandles;
"[SCENE]: Removing {0} agent {1} {2} from region {3}", regions.Remove(RegionInfo.RegionHandle);
(isChildAgent ? "child" : "root"), avatar.Name, agentID, RegionInfo.RegionName); m_sceneGridService.SendCloseChildAgentConnections(agentID, regions);
}
m_sceneGraph.removeUserCount(!isChildAgent); m_eventManager.TriggerClientClosed(agentID, this);
m_eventManager.TriggerOnRemovePresence(agentID);
// TODO: We shouldn't use closeChildAgents here - it's being used by the NPC module to stop if (!isChildAgent)
// unnecessary operations. This should go away once NPCs have no accompanying IClientAPI {
if (closeChildAgents && CapsModule != null) if (AttachmentsModule != null && avatar.PresenceType != PresenceType.Npc)
CapsModule.RemoveCaps(agentID);
// REFACTORING PROBLEM -- well not really a problem, but just to point out that whatever
// this method is doing is HORRIBLE!!!
avatar.Scene.NeedSceneCacheClear(avatar.UUID);
if (closeChildAgents && !avatar.IsChildAgent)
{ {
List<ulong> regions = avatar.KnownRegionHandles; IUserManagement uMan = RequestModuleInterface<IUserManagement>();
regions.Remove(RegionInfo.RegionHandle); // Don't save attachments for HG visitors, it
m_sceneGridService.SendCloseChildAgentConnections(agentID, regions); // messes up their inventory. When a HG visitor logs
// out on a foreign grid, their attachments will be
// reloaded in the state they were in when they left
// the home grid. This is best anyway as the visited
// grid may use an incompatible script engine.
if (uMan == null || uMan.IsLocalGridUser(avatar.UUID))
AttachmentsModule.SaveChangedAttachments(avatar, false);
} }
m_eventManager.TriggerClientClosed(agentID, this); ForEachClient(
} delegate(IClientAPI client)
catch (NullReferenceException)
{
// We don't know which count to remove it from
// Avatar is already disposed :/
}
try
{
m_eventManager.TriggerOnRemovePresence(agentID);
if (!isChildAgent)
{
if (AttachmentsModule != null && avatar.PresenceType != PresenceType.Npc)
{ {
IUserManagement uMan = RequestModuleInterface<IUserManagement>(); //We can safely ignore null reference exceptions. It means the avatar is dead and cleaned up anyway
// Don't save attachments for HG visitors, it try { client.SendKillObject(avatar.RegionHandle, new List<uint> { avatar.LocalId }); }
// messes up their inventory. When a HG visitor logs catch (NullReferenceException) { }
// out on a foreign grid, their attachments will be });
// reloaded in the state they were in when they left
// the home grid. This is best anyway as the visited
// grid may use an incompatible script engine.
if (uMan == null || uMan.IsLocalGridUser(avatar.UUID))
AttachmentsModule.SaveChangedAttachments(avatar, false);
}
ForEachClient(
delegate(IClientAPI client)
{
//We can safely ignore null reference exceptions. It means the avatar is dead and cleaned up anyway
try { client.SendKillObject(avatar.RegionHandle, new List<uint> { avatar.LocalId }); }
catch (NullReferenceException) { }
});
}
// It's possible for child agents to have transactions if changes are being made cross-border.
if (AgentTransactionsModule != null)
AgentTransactionsModule.RemoveAgentAssetTransactions(agentID);
}
finally
{
// Always clean these structures up so that any failure above doesn't cause them to remain in the
// scene with possibly bad effects (e.g. continually timing out on unacked packets and triggering
// the same cleanup exception continually.
// TODO: This should probably extend to the whole method, but we don't want to also catch the NRE
// since this would hide the underlying failure and other associated problems.
m_sceneGraph.RemoveScenePresence(agentID);
m_clientManager.Remove(agentID);
} }
try // It's possible for child agents to have transactions if changes are being made cross-border.
{ if (AgentTransactionsModule != null)
avatar.Close(); AgentTransactionsModule.RemoveAgentAssetTransactions(agentID);
}
catch (NullReferenceException) avatar.Close();
{
//We can safely ignore null reference exceptions. It means the avatar are dead and cleaned up anyway.
}
catch (Exception e)
{
m_log.ErrorFormat("[SCENE] Scene.cs:RemoveClient exception {0}{1}", e.Message, e.StackTrace);
}
m_authenticateHandler.RemoveCircuit(avatar.ControllingClient.CircuitCode); m_authenticateHandler.RemoveCircuit(avatar.ControllingClient.CircuitCode);
// CleanDroppedAttachments();
//m_log.InfoFormat("[SCENE] Memory pre GC {0}", System.GC.GetTotalMemory(false));
//m_log.InfoFormat("[SCENE] Memory post GC {0}", System.GC.GetTotalMemory(true));
} }
catch (Exception e)
{
m_log.Error(
string.Format("[SCENE]: Exception removing {0} from {1}, ", avatar.Name, RegionInfo.RegionName), e);
}
finally
{
// Always clean these structures up so that any failure above doesn't cause them to remain in the
// scene with possibly bad effects (e.g. continually timing out on unacked packets and triggering
// the same cleanup exception continually.
// TODO: This should probably extend to the whole method, but we don't want to also catch the NRE
// since this would hide the underlying failure and other associated problems.
m_sceneGraph.RemoveScenePresence(agentID);
m_clientManager.Remove(agentID);
}
//m_log.InfoFormat("[SCENE] Memory pre GC {0}", System.GC.GetTotalMemory(false));
//m_log.InfoFormat("[SCENE] Memory post GC {0}", System.GC.GetTotalMemory(true));
} }
/// <summary> /// <summary>

View File

@ -3437,7 +3437,7 @@ namespace OpenSim.Region.Framework.Scenes
public void Close() public void Close()
{ {
if (!IsChildAgent) if (!IsChildAgent && m_scene.AttachmentsModule != null)
m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false); m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false);
// Clear known regions // Clear known regions

View File

@ -101,7 +101,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
public void TestCloseAgent() public void TestCloseAgent()
{ {
TestHelpers.InMethod(); TestHelpers.InMethod();
// log4net.Config.XmlConfigurator.Configure(); // TestHelpers.EnableLogging();
TestScene scene = new SceneHelpers().SetupScene(); TestScene scene = new SceneHelpers().SetupScene();
ScenePresence sp = SceneHelpers.AddScenePresence(scene, TestHelpers.ParseTail(0x1)); ScenePresence sp = SceneHelpers.AddScenePresence(scene, TestHelpers.ParseTail(0x1));
@ -114,6 +114,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests
Assert.That(scene.GetScenePresence(sp.UUID), Is.Null); Assert.That(scene.GetScenePresence(sp.UUID), Is.Null);
Assert.That(scene.AuthenticateHandler.GetAgentCircuitData(sp.UUID), Is.Null); Assert.That(scene.AuthenticateHandler.GetAgentCircuitData(sp.UUID), Is.Null);
Assert.That(scene.AuthenticateHandler.GetAgentCircuits().Count, Is.EqualTo(0)); Assert.That(scene.AuthenticateHandler.GetAgentCircuits().Count, Is.EqualTo(0));
// TestHelpers.DisableLogging();
} }
[Test] [Test]