Do not use the SP.DoNotCloseAfterTeleport flag for child agent connections.

This approach has problems if a client quits without sending a proper logout but then reconnects before the connection is closed due to inactivity.
In this case, the DoNotCloseAfterTeleport was wrongly set.
The simplest approach is to close child agents on teleport as quickly as possible so that races are very unlikely to occur
Hence, this code now closes child agents as the first action after a sucessful teleport.

2 files changed, 32 insertions, 23 deletions

diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
index 87f0264..93a089d 100644
--- a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
+++ b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
@@ -1055,6 +1055,14 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
             
             m_entityTransferStateMachine.UpdateInTransit(sp.UUID, AgentTransferState.CleaningUp);
 
+            // Need to signal neighbours whether child agents may need closing irrespective of whether this
+            // one needed closing.  We also need to close child agents as quickly as possible to avoid complicated
+            // race conditions with rapid agent releporting (e.g. from A1 to a non-neighbour B, back
+            // to a neighbour A2 then off to a non-neighbour C).  Closing child agents any later requires complex
+            // distributed checks to avoid problems in rapid reteleporting scenarios and where child agents are
+            // abandoned without proper close by viewer but then re-used by an incoming connection.
+            sp.CloseChildAgents(newRegionX, newRegionY);
+
             // May need to logout or other cleanup
             AgentHasMovedAway(sp, logout);
 
@@ -1064,17 +1072,6 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
             // Now let's make it officially a child agent
             sp.MakeChildAgent();
 
-            // May still need to signal neighbours whether child agents may need closing irrespective of whether this
-            // one needed closing.  Neighbour regions also contain logic to prevent a close if a subsequent move or
-            // teleport re-established the child connection.
-            // 
-            // It may be possible to also close child agents after a pause but one needs to be very careful about
-            // race conditions between different regions on rapid teleporting (e.g. from A1 to a non-neighbour B, back
-            // to a neighbour A2 then off to a non-neighbour C.  Also, closing child agents early may be more compatible
-            // with complicated scenarios where there a mixture of V1 and V2 teleports, though this is conjecture.  It's
-            // easier to close immediately and greatly reduce the scope of race conditions if possible.
-            sp.CloseChildAgents(newRegionX, newRegionY);
-
             // Finally, let's close this previously-known-as-root agent, when the jump is outside the view zone
             if (NeedsClosing(sp.DrawDistance, oldRegionX, newRegionX, oldRegionY, newRegionY, reg))
             {
@@ -1096,7 +1093,10 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 }
                 else
                 {
-                    m_log.DebugFormat("[ENTITY TRANSFER MODULE]: Not closing agent {0}, user is back in {1}", sp.Name, Scene.Name);
+                    m_log.DebugFormat(
+                        "[ENTITY TRANSFER MODULE]: Connection for {0} in {1} has been re-established after teleport.  Not closing.", 
+                        sp.Name, Scene.Name);
+
                     sp.DoNotCloseAfterTeleport = false;
                 }
             }
diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs
index d187377..3e5ef10 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.cs
@@ -3703,21 +3703,30 @@ namespace OpenSim.Region.Framework.Scenes
                     // In the case where, for example, an A B C D region layout, an avatar may
                     // teleport from A -> D, but then -> C before A has asked B to close its old child agent.  When C
                     // renews the lease on the child agent at B, we must make sure that the close from A does not succeed.
-                    if (!acd.ChildrenCapSeeds.ContainsKey(RegionInfo.RegionHandle))
-                    {
-                        m_log.DebugFormat(
-                            "[SCENE]: Setting DoNotCloseAfterTeleport for child scene presence {0} in {1} because source will attempt close.", 
-                            sp.Name, Name);
+                    //
+                    // XXX: In the end, this should not be necessary if child agents are closed without delay on 
+                    // teleport, since realistically, the close request should always be processed before any other
+                    // region tried to re-establish a child agent.  This is much simpler since the logic below is 
+                    // vulnerable to an issue when a viewer quits a region without sending a proper logout but then
+                    // re-establishes the connection on a relogin.  This could wrongly set the DoNotCloseAfterTeleport
+                    // flag when no teleport had taken place (and hence no close was going to come).
+//                    if (!acd.ChildrenCapSeeds.ContainsKey(RegionInfo.RegionHandle))
+//                    {
+//                        m_log.DebugFormat(
+//                            "[SCENE]: Setting DoNotCloseAfterTeleport for child scene presence {0} in {1} because source will attempt close.", 
+//                            sp.Name, Name);
+//
+//                        sp.DoNotCloseAfterTeleport = true;
+//                    }
+//                    else if (EntityTransferModule.IsInTransit(sp.UUID))
 
-                        sp.DoNotCloseAfterTeleport = true;
-                    }
-                    else if (EntityTransferModule.IsInTransit(sp.UUID))
+                    if (EntityTransferModule.IsInTransit(sp.UUID))
                     {
+                        sp.DoNotCloseAfterTeleport = true;
+
                         m_log.DebugFormat(
-                            "[SCENE]: Setting DoNotCloseAfterTeleport for child scene presence {0} in {1} because this region will attempt previous end-of-teleport close.", 
+                            "[SCENE]: Set DoNotCloseAfterTeleport for child scene presence {0} in {1} because this region will attempt previous end-of-teleport close.", 
                             sp.Name, Name);
-
-                        sp.DoNotCloseAfterTeleport = true;
                     }
                 }
             }