diff --git a/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/EventQueueGetModule.cs b/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/EventQueueGetModule.cs
index 594b229f20..0dd0904986 100644
--- a/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/EventQueueGetModule.cs
+++ b/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/EventQueueGetModule.cs
@@ -94,7 +94,7 @@ namespace OpenSim.Region.ClientStack.Linden
//scene.CommsManager.HttpServer.AddLLSDHandler("/CAPS/EQG/", EventQueueFallBack);
- scene.EventManager.OnNewClient += OnNewClient;
+// scene.EventManager.OnNewClient += OnNewClient;
// TODO: Leaving these open, or closing them when we
// become a child is incorrect. It messes up TP in a big
@@ -102,6 +102,7 @@ namespace OpenSim.Region.ClientStack.Linden
// circuit is there.
scene.EventManager.OnClientClosed += ClientClosed;
+
scene.EventManager.OnMakeChildAgent += MakeChildAgent;
scene.EventManager.OnRegisterCaps += OnRegisterCaps;
@@ -226,16 +227,6 @@ namespace OpenSim.Region.ClientStack.Linden
#endregion
- private void OnNewClient(IClientAPI client)
- {
- //client.OnLogout += ClientClosed;
- }
-
-// private void ClientClosed(IClientAPI client)
-// {
-// ClientClosed(client.AgentId);
-// }
-
private void ClientClosed(UUID agentID, Scene scene)
{
// m_log.DebugFormat("[EVENTQUEUE]: Closed client {0} in region {1}", agentID, m_scene.RegionInfo.RegionName);
diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs
index d11fcbf7e0..ab670a7b1d 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs
@@ -1103,20 +1103,20 @@ namespace OpenSim.Region.ClientStack.LindenUDP
{
IClientAPI client = null;
- // In priciple there shouldn't be more than one thread here, ever.
- // But in case that happens, we need to synchronize this piece of code
- // because it's too important
- lock (this)
+ // We currently synchronize this code across the whole scene to avoid issues such as
+ // http://opensimulator.org/mantis/view.php?id=5365 However, once locking per agent circuit can be done
+ // consistently, this lock could probably be removed.
+ lock (this)
{
if (!m_scene.TryGetClient(agentID, out client))
{
LLUDPClient udpClient = new LLUDPClient(this, ThrottleRates, m_throttle, circuitCode, agentID, remoteEndPoint, m_defaultRTO, m_maxRTO);
-
+
client = new LLClientView(m_scene, this, udpClient, sessionInfo, agentID, sessionID, circuitCode);
client.OnLogout += LogoutHandler;
-
+
((LLClientView)client).DisableFacelights = m_disableFacelights;
-
+
client.Start();
}
}
diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs
index 25c95af430..751c461180 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.cs
@@ -79,6 +79,11 @@ namespace OpenSim.Region.Framework.Scenes
public SynchronizeSceneHandler SynchronizeScene;
+ ///
+ /// Used to prevent simultaneous calls to RemoveClient() for the same agent from interfering with each other.
+ ///
+ private object m_removeClientLock = new object();
+
///
/// Statistical information for this scene.
///
@@ -2658,69 +2663,89 @@ namespace OpenSim.Region.Framework.Scenes
public override ISceneAgent AddNewClient(IClientAPI client, PresenceType type)
{
+ ScenePresence sp;
+ bool vialogin;
+
// Validation occurs in LLUDPServer
+ //
+ // XXX: A race condition exists here where two simultaneous calls to AddNewClient can interfere with
+ // each other. In practice, this does not currently occur in the code.
AgentCircuitData aCircuit = m_authenticateHandler.GetAgentCircuitData(client.CircuitCode);
- bool vialogin
- = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0
- || (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0;
-
-// CheckHeartbeat();
-
- ScenePresence sp = GetScenePresence(client.AgentId);
-
- // XXX: Not sure how good it is to add a new client if a scene presence already exists. Possibly this
- // could occur if a viewer crashes and relogs before the old client is kicked out. But this could cause
- // other problems, and possible the code calling AddNewClient() should ensure that no client is already
- // connected.
- if (sp == null)
+ // We lock here on AgentCircuitData to prevent a race condition between the thread adding a new connection
+ // and a simultaneous one that removes it (as can happen if the client is closed at a particular point
+ // whilst connecting).
+ //
+ // It would be easier to lock across all NewUserConnection(), AddNewClient() and
+ // RemoveClient() calls for all agents, but this would allow a slow call (e.g. because of slow service
+ // response in some module listening to AddNewClient()) from holding up unrelated agent calls.
+ //
+ // In practice, the lock (this) in LLUDPServer.AddNewClient() currently lock across all
+ // AddNewClient() operations (though not other ops).
+ // In the future this can be relieved once locking per agent (not necessarily on AgentCircuitData) is improved.
+ lock (aCircuit)
{
- m_log.DebugFormat(
- "[SCENE]: Adding new child scene presence {0} {1} to scene {2} at pos {3}",
- client.Name, client.AgentId, RegionInfo.RegionName, client.StartPos);
+ vialogin
+ = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0
+ || (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0;
+
+ // CheckHeartbeat();
+
+ sp = GetScenePresence(client.AgentId);
- m_clientManager.Add(client);
- SubscribeToClientEvents(client);
-
- sp = m_sceneGraph.CreateAndAddChildScenePresence(client, aCircuit.Appearance, type);
- m_eventManager.TriggerOnNewPresence(sp);
-
- sp.TeleportFlags = (TPFlags)aCircuit.teleportFlags;
-
- // The first agent upon login is a root agent by design.
- // For this agent we will have to rez the attachments.
- // All other AddNewClient calls find aCircuit.child to be true.
- if (aCircuit.child == false)
+ // XXX: Not sure how good it is to add a new client if a scene presence already exists. Possibly this
+ // could occur if a viewer crashes and relogs before the old client is kicked out. But this could cause
+ // other problems, and possible the code calling AddNewClient() should ensure that no client is already
+ // connected.
+ if (sp == null)
{
- // We have to set SP to be a root agent here so that SP.MakeRootAgent() will later not try to
- // start the scripts again (since this is done in RezAttachments()).
- // XXX: This is convoluted.
- sp.IsChildAgent = false;
-
- if (AttachmentsModule != null)
- Util.FireAndForget(delegate(object o) { AttachmentsModule.RezAttachments(sp); });
+ m_log.DebugFormat(
+ "[SCENE]: Adding new child scene presence {0} {1} to scene {2} at pos {3}",
+ client.Name, client.AgentId, RegionInfo.RegionName, client.StartPos);
+
+ m_clientManager.Add(client);
+ SubscribeToClientEvents(client);
+
+ sp = m_sceneGraph.CreateAndAddChildScenePresence(client, aCircuit.Appearance, type);
+ m_eventManager.TriggerOnNewPresence(sp);
+
+ sp.TeleportFlags = (TPFlags)aCircuit.teleportFlags;
+
+ // The first agent upon login is a root agent by design.
+ // For this agent we will have to rez the attachments.
+ // All other AddNewClient calls find aCircuit.child to be true.
+ if (aCircuit.child == false)
+ {
+ // We have to set SP to be a root agent here so that SP.MakeRootAgent() will later not try to
+ // start the scripts again (since this is done in RezAttachments()).
+ // XXX: This is convoluted.
+ sp.IsChildAgent = false;
+
+ if (AttachmentsModule != null)
+ Util.FireAndForget(delegate(object o) { AttachmentsModule.RezAttachments(sp); });
+ }
}
- }
- else
- {
- m_log.WarnFormat(
- "[SCENE]: Already found {0} scene presence for {1} in {2} when asked to add new scene presence",
- sp.IsChildAgent ? "child" : "root", sp.Name, RegionInfo.RegionName);
- }
+ else
+ {
+ m_log.WarnFormat(
+ "[SCENE]: Already found {0} scene presence for {1} in {2} when asked to add new scene presence",
+ sp.IsChildAgent ? "child" : "root", sp.Name, RegionInfo.RegionName);
+ }
+
+ // We must set this here so that TriggerOnNewClient and TriggerOnClientLogin can determine whether the
+ // client is for a root or child agent.
+ client.SceneAgent = sp;
- // We must set this here so that TriggerOnNewClient and TriggerOnClientLogin can determine whether the
- // client is for a root or child agent.
- client.SceneAgent = sp;
+ // Cache the user's name
+ CacheUserName(sp, aCircuit);
+
+ EventManager.TriggerOnNewClient(client);
+ if (vialogin)
+ EventManager.TriggerOnClientLogin(client);
+ }
m_LastLogin = Util.EnvironmentTickCount();
- // Cache the user's name
- CacheUserName(sp, aCircuit);
-
- EventManager.TriggerOnNewClient(client);
- if (vialogin)
- EventManager.TriggerOnClientLogin(client);
-
return sp;
}
@@ -3249,109 +3274,129 @@ namespace OpenSim.Region.Framework.Scenes
{
// CheckHeartbeat();
bool isChildAgent = false;
- ScenePresence avatar = GetScenePresence(agentID);
+ AgentCircuitData acd;
- if (avatar == null)
+ lock (m_removeClientLock)
{
- m_log.WarnFormat(
- "[SCENE]: Called RemoveClient() with agent ID {0} but no such presence is in the scene.", agentID);
+ acd = m_authenticateHandler.GetAgentCircuitData(agentID);
- return;
+ if (acd == null)
+ {
+ m_log.ErrorFormat("[SCENE]: No agent circuit found for {0}, aborting Scene.RemoveClient", agentID);
+ return;
+ }
+ else
+ {
+ // We remove the acd up here to avoid later raec conditions if two RemoveClient() calls occurred
+ // simultaneously.
+ m_authenticateHandler.RemoveCircuit(acd.circuitcode);
+ }
}
- try
+ lock (acd)
{
- 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
- if (closeChildAgents && isChildAgent)
+ ScenePresence avatar = GetScenePresence(agentID);
+
+ if (avatar == null)
{
- // Tell a single agent to disconnect from the region.
- IEventQueue eq = RequestModuleInterface();
- if (eq != null)
- {
- eq.DisableSimulator(RegionInfo.RegionHandle, avatar.UUID);
- }
- else
- {
- avatar.ControllingClient.SendShutdownConnectionNotice();
- }
+ m_log.WarnFormat(
+ "[SCENE]: Called RemoveClient() with agent ID {0} but no such presence is in the scene.", agentID);
+
+ return;
}
-
- // Only applies to root agents.
- if (avatar.ParentID != 0)
- {
- avatar.StandUp();
- }
-
- 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)
- {
- List regions = avatar.KnownRegionHandles;
- regions.Remove(RegionInfo.RegionHandle);
- m_sceneGridService.SendCloseChildAgentConnections(agentID, regions);
- }
-
- m_eventManager.TriggerClientClosed(agentID, this);
- m_eventManager.TriggerOnRemovePresence(agentID);
-
- if (!isChildAgent)
- {
- if (AttachmentsModule != null)
- {
- AttachmentsModule.DeRezAttachments(avatar);
- }
-
- 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 { 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);
-
- m_authenticateHandler.RemoveCircuit(avatar.ControllingClient.CircuitCode);
- }
- catch (Exception e)
- {
- m_log.Error(
- string.Format("[SCENE]: Exception removing {0} from {1}. Cleaning up. Exception ", avatar.Name, Name), e);
- }
- finally
- {
+
try
{
- // 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.
- m_sceneGraph.RemoveScenePresence(agentID);
- m_clientManager.Remove(agentID);
+ isChildAgent = avatar.IsChildAgent;
+
+ m_log.DebugFormat(
+ "[SCENE]: Removing {0} agent {1} {2} from {3}",
+ (isChildAgent ? "child" : "root"), avatar.Name, agentID, RegionInfo.RegionName);
- avatar.Close();
+ // Don't do this to root agents, it's not nice for the viewer
+ if (closeChildAgents && isChildAgent)
+ {
+ // Tell a single agent to disconnect from the region.
+ IEventQueue eq = RequestModuleInterface();
+ if (eq != null)
+ {
+ eq.DisableSimulator(RegionInfo.RegionHandle, avatar.UUID);
+ }
+ else
+ {
+ avatar.ControllingClient.SendShutdownConnectionNotice();
+ }
+ }
+
+ // Only applies to root agents.
+ if (avatar.ParentID != 0)
+ {
+ avatar.StandUp();
+ }
+
+ 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)
+ {
+ List regions = avatar.KnownRegionHandles;
+ regions.Remove(RegionInfo.RegionHandle);
+ m_sceneGridService.SendCloseChildAgentConnections(agentID, regions);
+ }
+
+ m_eventManager.TriggerClientClosed(agentID, this);
+ m_eventManager.TriggerOnRemovePresence(agentID);
+
+ if (!isChildAgent)
+ {
+ if (AttachmentsModule != null)
+ {
+ AttachmentsModule.DeRezAttachments(avatar);
+ }
+
+ 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 { 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);
}
catch (Exception e)
{
m_log.Error(
- string.Format("[SCENE]: Exception in final clean up of {0} in {1}. Exception ", avatar.Name, Name), e);
+ string.Format("[SCENE]: Exception removing {0} from {1}. Cleaning up. Exception ", avatar.Name, Name), e);
+ }
+ finally
+ {
+ try
+ {
+ // 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.
+ m_sceneGraph.RemoveScenePresence(agentID);
+ m_clientManager.Remove(agentID);
+
+ avatar.Close();
+ }
+ catch (Exception e)
+ {
+ m_log.Error(
+ string.Format("[SCENE]: Exception in final clean up of {0} in {1}. Exception ", avatar.Name, Name), e);
+ }
}
}
@@ -3521,87 +3566,97 @@ namespace OpenSim.Region.Framework.Scenes
agent.firstname, agent.lastname, agent.Viewer);
reason = "Access denied, your viewer is banned by the region owner";
return false;
- }
-
- ScenePresence sp = GetScenePresence(agent.AgentID);
-
- if (sp != null && !sp.IsChildAgent)
- {
- // We have a zombie from a crashed session.
- // Or the same user is trying to be root twice here, won't work.
- // Kill it.
- m_log.WarnFormat(
- "[SCENE]: Existing root scene presence detected for {0} {1} in {2} when connecting. Removing existing presence.",
- sp.Name, sp.UUID, RegionInfo.RegionName);
-
- sp.ControllingClient.Close(true);
- sp = null;
}
- ILandObject land = LandChannel.GetLandObject(agent.startpos.X, agent.startpos.Y);
+ ILandObject land;
- //On login test land permisions
- if (vialogin)
+ lock (agent)
{
- if (land != null && !TestLandRestrictions(agent, land, out reason))
+ ScenePresence sp = GetScenePresence(agent.AgentID);
+
+ if (sp != null && !sp.IsChildAgent)
{
- return false;
+ // We have a zombie from a crashed session.
+ // Or the same user is trying to be root twice here, won't work.
+ // Kill it.
+ m_log.WarnFormat(
+ "[SCENE]: Existing root scene presence detected for {0} {1} in {2} when connecting. Removing existing presence.",
+ sp.Name, sp.UUID, RegionInfo.RegionName);
+
+ sp.ControllingClient.Close(true);
+ sp = null;
}
- }
-
- if (sp == null) // We don't have an [child] agent here already
- {
- if (requirePresenceLookup)
+
+ land = LandChannel.GetLandObject(agent.startpos.X, agent.startpos.Y);
+
+ //On login test land permisions
+ if (vialogin)
{
- try
+ if (land != null && !TestLandRestrictions(agent, land, out reason))
{
- if (!VerifyUserPresence(agent, out reason))
- return false;
- } catch (Exception e)
- {
- m_log.ErrorFormat(
- "[SCENE]: Exception verifying presence {0}{1}", e.Message, e.StackTrace);
return false;
}
}
-
- try
+
+ if (sp == null) // We don't have an [child] agent here already
{
- if (!AuthorizeUser(agent, out reason))
+ if (requirePresenceLookup)
+ {
+ try
+ {
+ if (!VerifyUserPresence(agent, out reason))
+ return false;
+ }
+ catch (Exception e)
+ {
+ m_log.ErrorFormat(
+ "[SCENE]: Exception verifying presence {0}{1}", e.Message, e.StackTrace);
+
+ return false;
+ }
+ }
+
+ try
+ {
+ if (!AuthorizeUser(agent, out reason))
+ return false;
+ }
+ catch (Exception e)
+ {
+ m_log.ErrorFormat(
+ "[SCENE]: Exception authorizing user {0}{1}", e.Message, e.StackTrace);
+
return false;
- } catch (Exception e)
- {
- m_log.ErrorFormat(
- "[SCENE]: Exception authorizing user {0}{1}", e.Message, e.StackTrace);
- return false;
- }
-
- m_log.InfoFormat(
- "[SCENE]: Region {0} authenticated and authorized incoming {1} agent {2} {3} {4} (circuit code {5})",
- RegionInfo.RegionName, (agent.child ? "child" : "root"), agent.firstname, agent.lastname,
- agent.AgentID, agent.circuitcode);
-
- if (CapsModule != null)
- {
- CapsModule.SetAgentCapsSeeds(agent);
- CapsModule.CreateCaps(agent.AgentID);
- }
- } else
- {
- // Let the SP know how we got here. This has a lot of interesting
- // uses down the line.
- sp.TeleportFlags = (TPFlags)teleportFlags;
-
- if (sp.IsChildAgent)
- {
- m_log.DebugFormat(
- "[SCENE]: Adjusting known seeds for existing agent {0} in {1}",
- agent.AgentID, RegionInfo.RegionName);
-
- sp.AdjustKnownSeeds();
-
+ }
+
+ m_log.InfoFormat(
+ "[SCENE]: Region {0} authenticated and authorized incoming {1} agent {2} {3} {4} (circuit code {5})",
+ RegionInfo.RegionName, (agent.child ? "child" : "root"), agent.firstname, agent.lastname,
+ agent.AgentID, agent.circuitcode);
+
if (CapsModule != null)
+ {
CapsModule.SetAgentCapsSeeds(agent);
+ CapsModule.CreateCaps(agent.AgentID);
+ }
+ }
+ else
+ {
+ // Let the SP know how we got here. This has a lot of interesting
+ // uses down the line.
+ sp.TeleportFlags = (TPFlags)teleportFlags;
+
+ if (sp.IsChildAgent)
+ {
+ m_log.DebugFormat(
+ "[SCENE]: Adjusting known seeds for existing agent {0} in {1}",
+ agent.AgentID, RegionInfo.RegionName);
+
+ sp.AdjustKnownSeeds();
+
+ if (CapsModule != null)
+ CapsModule.SetAgentCapsSeeds(agent);
+ }
}
}