Lock on AgentCircuitData during Scene.AddClient() and RemoveClient() to prevent an inactive connection being left behind if the user closes the viewer whilst the connection is being established.

This should remove the need to run the console command "kick user --force" when these connections are left around.
0.7.3-extended
Justin Clark-Casey (justincc) 2012-10-10 00:26:43 +01:00
parent e54c11e0da
commit 2271986396
3 changed files with 260 additions and 228 deletions

View File

@ -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);

View File

@ -1094,9 +1094,9 @@ 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
// 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))

View File

@ -78,6 +78,11 @@ namespace OpenSim.Region.Framework.Scenes
public SynchronizeSceneHandler SynchronizeScene;
/// <summary>
/// Used to prevent simultaneous calls to RemoveClient() for the same agent from interfering with each other.
/// </summary>
private object m_removeClientLock = new object();
/// <summary>
/// Statistical information for this scene.
/// </summary>
@ -2594,16 +2599,35 @@ 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
// 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)
{
vialogin
= (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0
|| (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0;
// CheckHeartbeat();
// CheckHeartbeat();
ScenePresence sp = GetScenePresence(client.AgentId);
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
@ -2648,22 +2672,15 @@ namespace OpenSim.Region.Framework.Scenes
// client is for a root or child agent.
client.SceneAgent = sp;
m_LastLogin = Util.EnvironmentTickCount();
// Cache the user's name
CacheUserName(sp, aCircuit);
// Let's send the Suitcase folder for incoming HG agents
if ((aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0)
{
m_log.DebugFormat("[SCENE]: Sending root folder to viewer...");
InventoryFolderBase suitcase = InventoryService.GetRootFolder(client.AgentId);
client.SendBulkUpdateInventory(suitcase);
}
EventManager.TriggerOnNewClient(client);
if (vialogin)
EventManager.TriggerOnClientLogin(client);
}
m_LastLogin = Util.EnvironmentTickCount();
return sp;
}
@ -3195,6 +3212,27 @@ namespace OpenSim.Region.Framework.Scenes
{
// CheckHeartbeat();
bool isChildAgent = false;
AgentCircuitData acd;
lock (m_removeClientLock)
{
acd = m_authenticateHandler.GetAgentCircuitData(agentID);
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);
}
}
lock (acd)
{
ScenePresence avatar = GetScenePresence(agentID);
if (avatar == null)
@ -3256,8 +3294,6 @@ namespace OpenSim.Region.Framework.Scenes
m_eventManager.TriggerOnRemovePresence(agentID);
if (!isChildAgent)
{
if (AttachmentsModule != null)
{
// Don't save attachments for HG visitors, it
// messes up their inventory. When a HG visitor logs
@ -3270,7 +3306,6 @@ namespace OpenSim.Region.Framework.Scenes
&& (UserManagementModule == null || UserManagementModule.IsLocalGridUser(avatar.UUID));
AttachmentsModule.DeRezAttachments(avatar, saveChanged, false);
}
ForEachClient(
delegate(IClientAPI client)
@ -3284,8 +3319,6 @@ namespace OpenSim.Region.Framework.Scenes
// 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)
{
@ -3310,6 +3343,7 @@ namespace OpenSim.Region.Framework.Scenes
string.Format("[SCENE]: Exception in final clean up of {0} in {1}. Exception ", avatar.Name, Name), e);
}
}
}
//m_log.InfoFormat("[SCENE] Memory pre GC {0}", System.GC.GetTotalMemory(false));
//m_log.InfoFormat("[SCENE] Memory post GC {0}", System.GC.GetTotalMemory(true));
@ -3419,6 +3453,10 @@ namespace OpenSim.Region.Framework.Scenes
return false;
}
ILandObject land;
lock (agent)
{
ScenePresence sp = GetScenePresence(agent.AgentID);
if (sp != null && !sp.IsChildAgent)
@ -3430,11 +3468,11 @@ namespace OpenSim.Region.Framework.Scenes
"[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();
sp.ControllingClient.Close(true);
sp = null;
}
ILandObject land = LandChannel.GetLandObject(agent.startpos.X, agent.startpos.Y);
land = LandChannel.GetLandObject(agent.startpos.X, agent.startpos.Y);
//On login test land permisions
if (vialogin)
@ -3458,6 +3496,7 @@ namespace OpenSim.Region.Framework.Scenes
{
m_log.ErrorFormat(
"[SCENE]: Exception verifying presence {0}{1}", e.Message, e.StackTrace);
return false;
}
}
@ -3471,6 +3510,7 @@ namespace OpenSim.Region.Framework.Scenes
{
m_log.ErrorFormat(
"[SCENE]: Exception authorizing user {0}{1}", e.Message, e.StackTrace);
return false;
}
@ -3503,6 +3543,7 @@ namespace OpenSim.Region.Framework.Scenes
CapsModule.SetAgentCapsSeeds(agent);
}
}
}
// In all cases, add or update the circuit data with the new agent circuit data and teleport flags
agent.teleportFlags = teleportFlags;