diff options
author | Justin Clark-Casey (justincc) | 2012-06-08 01:24:44 +0100 |
---|---|---|
committer | Justin Clark-Casey (justincc) | 2012-06-08 01:26:43 +0100 |
commit | 30f4a33f01751d151ae3923b48e6e98083791140 (patch) | |
tree | e361716ee927c731621276b8bc490924a71d277f /OpenSim/Region/Framework | |
parent | Go back to calling IncomingCloseAgent() in the "kick user" command for consis... (diff) | |
download | opensim-SC-30f4a33f01751d151ae3923b48e6e98083791140.zip opensim-SC-30f4a33f01751d151ae3923b48e6e98083791140.tar.gz opensim-SC-30f4a33f01751d151ae3923b48e6e98083791140.tar.bz2 opensim-SC-30f4a33f01751d151ae3923b48e6e98083791140.tar.xz |
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.
Diffstat (limited to 'OpenSim/Region/Framework')
-rw-r--r-- | OpenSim/Region/Framework/Scenes/Scene.cs | 149 | ||||
-rw-r--r-- | OpenSim/Region/Framework/Scenes/ScenePresence.cs | 2 | ||||
-rw-r--r-- | OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs | 4 |
3 files changed, 74 insertions, 81 deletions
diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 9048f00..7afde38 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs | |||
@@ -3231,10 +3231,23 @@ namespace OpenSim.Region.Framework.Scenes | |||
3231 | // CheckHeartbeat(); | 3231 | // CheckHeartbeat(); |
3232 | bool isChildAgent = false; | 3232 | bool isChildAgent = false; |
3233 | ScenePresence avatar = GetScenePresence(agentID); | 3233 | ScenePresence avatar = GetScenePresence(agentID); |
3234 | if (avatar != null) | 3234 | |
3235 | if (avatar == null) | ||
3236 | { | ||
3237 | m_log.WarnFormat( | ||
3238 | "[SCENE]: Called RemoveClient() with agent ID {0} but no such presence is in the scene.", agentID); | ||
3239 | |||
3240 | return; | ||
3241 | } | ||
3242 | |||
3243 | try | ||
3235 | { | 3244 | { |
3236 | isChildAgent = avatar.IsChildAgent; | 3245 | isChildAgent = avatar.IsChildAgent; |
3237 | 3246 | ||
3247 | m_log.DebugFormat( | ||
3248 | "[SCENE]: Removing {0} agent {1} {2} from {3}", | ||
3249 | (isChildAgent ? "child" : "root"), avatar.Name, agentID, RegionInfo.RegionName); | ||
3250 | |||
3238 | // Don't do this to root agents, it's not nice for the viewer | 3251 | // Don't do this to root agents, it's not nice for the viewer |
3239 | if (closeChildAgents && isChildAgent) | 3252 | if (closeChildAgents && isChildAgent) |
3240 | { | 3253 | { |
@@ -3256,99 +3269,77 @@ namespace OpenSim.Region.Framework.Scenes | |||
3256 | avatar.StandUp(); | 3269 | avatar.StandUp(); |
3257 | } | 3270 | } |
3258 | 3271 | ||
3259 | try | 3272 | m_sceneGraph.removeUserCount(!isChildAgent); |
3260 | { | ||
3261 | m_log.DebugFormat( | ||
3262 | "[SCENE]: Removing {0} agent {1} {2} from region {3}", | ||
3263 | (isChildAgent ? "child" : "root"), avatar.Name, agentID, RegionInfo.RegionName); | ||
3264 | |||
3265 | m_sceneGraph.removeUserCount(!isChildAgent); | ||
3266 | |||
3267 | // TODO: We shouldn't use closeChildAgents here - it's being used by the NPC module to stop | ||
3268 | // unnecessary operations. This should go away once NPCs have no accompanying IClientAPI | ||
3269 | if (closeChildAgents && CapsModule != null) | ||
3270 | CapsModule.RemoveCaps(agentID); | ||
3271 | 3273 | ||
3272 | // REFACTORING PROBLEM -- well not really a problem, but just to point out that whatever | 3274 | // TODO: We shouldn't use closeChildAgents here - it's being used by the NPC module to stop |
3273 | // this method is doing is HORRIBLE!!! | 3275 | // unnecessary operations. This should go away once NPCs have no accompanying IClientAPI |
3274 | avatar.Scene.NeedSceneCacheClear(avatar.UUID); | 3276 | if (closeChildAgents && CapsModule != null) |
3277 | CapsModule.RemoveCaps(agentID); | ||
3275 | 3278 | ||
3276 | if (closeChildAgents && !avatar.IsChildAgent) | 3279 | // REFACTORING PROBLEM -- well not really a problem, but just to point out that whatever |
3277 | { | 3280 | // this method is doing is HORRIBLE!!! |
3278 | List<ulong> regions = avatar.KnownRegionHandles; | 3281 | avatar.Scene.NeedSceneCacheClear(avatar.UUID); |
3279 | regions.Remove(RegionInfo.RegionHandle); | ||
3280 | m_sceneGridService.SendCloseChildAgentConnections(agentID, regions); | ||
3281 | } | ||
3282 | 3282 | ||
3283 | m_eventManager.TriggerClientClosed(agentID, this); | 3283 | if (closeChildAgents && !isChildAgent) |
3284 | } | ||
3285 | catch (NullReferenceException) | ||
3286 | { | 3284 | { |
3287 | // We don't know which count to remove it from | 3285 | List<ulong> regions = avatar.KnownRegionHandles; |
3288 | // Avatar is already disposed :/ | 3286 | regions.Remove(RegionInfo.RegionHandle); |
3287 | m_sceneGridService.SendCloseChildAgentConnections(agentID, regions); | ||
3289 | } | 3288 | } |
3290 | 3289 | ||
3291 | try | 3290 | m_eventManager.TriggerClientClosed(agentID, this); |
3291 | m_eventManager.TriggerOnRemovePresence(agentID); | ||
3292 | |||
3293 | if (!isChildAgent) | ||
3292 | { | 3294 | { |
3293 | m_eventManager.TriggerOnRemovePresence(agentID); | 3295 | if (AttachmentsModule != null && avatar.PresenceType != PresenceType.Npc) |
3294 | |||
3295 | if (!isChildAgent) | ||
3296 | { | 3296 | { |
3297 | if (AttachmentsModule != null && avatar.PresenceType != PresenceType.Npc) | 3297 | IUserManagement uMan = RequestModuleInterface<IUserManagement>(); |
3298 | { | 3298 | // Don't save attachments for HG visitors, it |
3299 | IUserManagement uMan = RequestModuleInterface<IUserManagement>(); | 3299 | // messes up their inventory. When a HG visitor logs |
3300 | // Don't save attachments for HG visitors, it | 3300 | // out on a foreign grid, their attachments will be |
3301 | // messes up their inventory. When a HG visitor logs | 3301 | // reloaded in the state they were in when they left |
3302 | // out on a foreign grid, their attachments will be | 3302 | // the home grid. This is best anyway as the visited |
3303 | // reloaded in the state they were in when they left | 3303 | // grid may use an incompatible script engine. |
3304 | // the home grid. This is best anyway as the visited | 3304 | if (uMan == null || uMan.IsLocalGridUser(avatar.UUID)) |
3305 | // grid may use an incompatible script engine. | 3305 | AttachmentsModule.SaveChangedAttachments(avatar, false); |
3306 | if (uMan == null || uMan.IsLocalGridUser(avatar.UUID)) | ||
3307 | AttachmentsModule.SaveChangedAttachments(avatar, false); | ||
3308 | } | ||
3309 | |||
3310 | ForEachClient( | ||
3311 | delegate(IClientAPI client) | ||
3312 | { | ||
3313 | //We can safely ignore null reference exceptions. It means the avatar is dead and cleaned up anyway | ||
3314 | try { client.SendKillObject(avatar.RegionHandle, new List<uint> { avatar.LocalId }); } | ||
3315 | catch (NullReferenceException) { } | ||
3316 | }); | ||
3317 | } | 3306 | } |
3318 | 3307 | ||
3319 | // It's possible for child agents to have transactions if changes are being made cross-border. | 3308 | ForEachClient( |
3320 | if (AgentTransactionsModule != null) | 3309 | delegate(IClientAPI client) |
3321 | AgentTransactionsModule.RemoveAgentAssetTransactions(agentID); | 3310 | { |
3322 | } | 3311 | //We can safely ignore null reference exceptions. It means the avatar is dead and cleaned up anyway |
3323 | finally | 3312 | try { client.SendKillObject(avatar.RegionHandle, new List<uint> { avatar.LocalId }); } |
3324 | { | 3313 | catch (NullReferenceException) { } |
3325 | // Always clean these structures up so that any failure above doesn't cause them to remain in the | 3314 | }); |
3326 | // scene with possibly bad effects (e.g. continually timing out on unacked packets and triggering | ||
3327 | // the same cleanup exception continually. | ||
3328 | // TODO: This should probably extend to the whole method, but we don't want to also catch the NRE | ||
3329 | // since this would hide the underlying failure and other associated problems. | ||
3330 | m_sceneGraph.RemoveScenePresence(agentID); | ||
3331 | m_clientManager.Remove(agentID); | ||
3332 | } | 3315 | } |
3333 | 3316 | ||
3334 | try | 3317 | // It's possible for child agents to have transactions if changes are being made cross-border. |
3335 | { | 3318 | if (AgentTransactionsModule != null) |
3336 | avatar.Close(); | 3319 | AgentTransactionsModule.RemoveAgentAssetTransactions(agentID); |
3337 | } | 3320 | |
3338 | catch (NullReferenceException) | 3321 | avatar.Close(); |
3339 | { | ||
3340 | //We can safely ignore null reference exceptions. It means the avatar are dead and cleaned up anyway. | ||
3341 | } | ||
3342 | catch (Exception e) | ||
3343 | { | ||
3344 | m_log.ErrorFormat("[SCENE] Scene.cs:RemoveClient exception {0}{1}", e.Message, e.StackTrace); | ||
3345 | } | ||
3346 | 3322 | ||
3347 | m_authenticateHandler.RemoveCircuit(avatar.ControllingClient.CircuitCode); | 3323 | m_authenticateHandler.RemoveCircuit(avatar.ControllingClient.CircuitCode); |
3348 | // CleanDroppedAttachments(); | ||
3349 | //m_log.InfoFormat("[SCENE] Memory pre GC {0}", System.GC.GetTotalMemory(false)); | ||
3350 | //m_log.InfoFormat("[SCENE] Memory post GC {0}", System.GC.GetTotalMemory(true)); | ||
3351 | } | 3324 | } |
3325 | catch (Exception e) | ||
3326 | { | ||
3327 | m_log.Error( | ||
3328 | string.Format("[SCENE]: Exception removing {0} from {1}, ", avatar.Name, RegionInfo.RegionName), e); | ||
3329 | } | ||
3330 | finally | ||
3331 | { | ||
3332 | // Always clean these structures up so that any failure above doesn't cause them to remain in the | ||
3333 | // scene with possibly bad effects (e.g. continually timing out on unacked packets and triggering | ||
3334 | // the same cleanup exception continually. | ||
3335 | // TODO: This should probably extend to the whole method, but we don't want to also catch the NRE | ||
3336 | // since this would hide the underlying failure and other associated problems. | ||
3337 | m_sceneGraph.RemoveScenePresence(agentID); | ||
3338 | m_clientManager.Remove(agentID); | ||
3339 | } | ||
3340 | |||
3341 | //m_log.InfoFormat("[SCENE] Memory pre GC {0}", System.GC.GetTotalMemory(false)); | ||
3342 | //m_log.InfoFormat("[SCENE] Memory post GC {0}", System.GC.GetTotalMemory(true)); | ||
3352 | } | 3343 | } |
3353 | 3344 | ||
3354 | /// <summary> | 3345 | /// <summary> |
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 3adafc1..029e0d7 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs | |||
@@ -3416,7 +3416,7 @@ namespace OpenSim.Region.Framework.Scenes | |||
3416 | 3416 | ||
3417 | public void Close() | 3417 | public void Close() |
3418 | { | 3418 | { |
3419 | if (!IsChildAgent) | 3419 | if (!IsChildAgent && m_scene.AttachmentsModule != null) |
3420 | m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false); | 3420 | m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false); |
3421 | 3421 | ||
3422 | // Clear known regions | 3422 | // Clear known regions |
diff --git a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs index 1aa48d7..02c45ef 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs | |||
@@ -101,7 +101,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests | |||
101 | public void TestCloseAgent() | 101 | public void TestCloseAgent() |
102 | { | 102 | { |
103 | TestHelpers.InMethod(); | 103 | TestHelpers.InMethod(); |
104 | // log4net.Config.XmlConfigurator.Configure(); | 104 | // TestHelpers.EnableLogging(); |
105 | 105 | ||
106 | TestScene scene = new SceneHelpers().SetupScene(); | 106 | TestScene scene = new SceneHelpers().SetupScene(); |
107 | ScenePresence sp = SceneHelpers.AddScenePresence(scene, TestHelpers.ParseTail(0x1)); | 107 | ScenePresence sp = SceneHelpers.AddScenePresence(scene, TestHelpers.ParseTail(0x1)); |
@@ -114,6 +114,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests | |||
114 | Assert.That(scene.GetScenePresence(sp.UUID), Is.Null); | 114 | Assert.That(scene.GetScenePresence(sp.UUID), Is.Null); |
115 | Assert.That(scene.AuthenticateHandler.GetAgentCircuitData(sp.UUID), Is.Null); | 115 | Assert.That(scene.AuthenticateHandler.GetAgentCircuitData(sp.UUID), Is.Null); |
116 | Assert.That(scene.AuthenticateHandler.GetAgentCircuits().Count, Is.EqualTo(0)); | 116 | Assert.That(scene.AuthenticateHandler.GetAgentCircuits().Count, Is.EqualTo(0)); |
117 | |||
118 | // TestHelpers.DisableLogging(); | ||
117 | } | 119 | } |
118 | 120 | ||
119 | [Test] | 121 | [Test] |