From 03993d0b14599358ac30a001cebd5b2145881c65 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Wed, 2 Nov 2011 18:12:12 +0000
Subject: Fix race condition that would sometimes send or save appearance for
 the wrong avatar.

In AvatarFactoryModule.HandleAppearanceUpdateTimer(), we loop through appearance save and send requests and dispatch via a FireAndForget thread.
If there was more than one request in the save or send queue, then this led to a subtle race condition where the foreach loop would load in the next KeyValuePair before the thread was dispatched.
This gave the thread the wrong avatar ID, leaving some avatar appearance cloudy since appearance data was never sent.
This change loads the fields into local references so that this doesn't happen.
---
 .../Avatar/AvatarFactory/AvatarFactoryModule.cs    | 28 ++++++++++++++++------
 OpenSim/Region/Framework/Scenes/ScenePresence.cs   |  5 ++--
 2 files changed, 24 insertions(+), 9 deletions(-)

(limited to 'OpenSim')

diff --git a/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs b/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
index 236a47c..07d1cb3 100644
--- a/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
@@ -169,6 +169,8 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
 
         public bool SendAppearance(UUID agentId)
         {
+//            m_log.DebugFormat("[AVFACTORY]: Sending appearance for {0}", agentId);
+
             ScenePresence sp = m_scene.GetScenePresence(agentId);
             if (sp == null)
             {
@@ -257,7 +259,7 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
         /// </summary>
         public void QueueAppearanceSend(UUID agentid)
         {
-            // m_log.WarnFormat("[AVFACTORY]: Queue appearance send for {0}", agentid);
+//            m_log.DebugFormat("[AVFACTORY]: Queue appearance send for {0}", agentid);
 
             // 10000 ticks per millisecond, 1000 milliseconds per second
             long timestamp = DateTime.Now.Ticks + Convert.ToInt64(m_sendtime * 1000 * 10000);
@@ -393,10 +395,17 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
                 Dictionary<UUID, long> sends = new Dictionary<UUID, long>(m_sendqueue);
                 foreach (KeyValuePair<UUID, long> kvp in sends)
                 {
-                    if (kvp.Value < now)
+                    // We have to load the key and value into local parameters to avoid a race condition if we loop
+                    // around and load kvp with a different value before FireAndForget has launched its thread.
+                    UUID avatarID = kvp.Key;
+                    long sendTime = kvp.Value;
+
+//                    m_log.DebugFormat("[AVFACTORY]: Handling queued appearance updates for {0}, update delta to now is {1}", avatarID, sendTime - now);
+
+                    if (sendTime < now)
                     {
-                        Util.FireAndForget(delegate(object o) { SendAppearance(kvp.Key); });
-                        m_sendqueue.Remove(kvp.Key);
+                        Util.FireAndForget(o => SendAppearance(avatarID));
+                        m_sendqueue.Remove(avatarID);
                     }
                 }
             }
@@ -406,10 +415,15 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
                 Dictionary<UUID, long> saves = new Dictionary<UUID, long>(m_savequeue);
                 foreach (KeyValuePair<UUID, long> kvp in saves)
                 {
-                    if (kvp.Value < now)
+                    // We have to load the key and value into local parameters to avoid a race condition if we loop
+                    // around and load kvp with a different value before FireAndForget has launched its thread.                    
+                    UUID avatarID = kvp.Key;
+                    long sendTime = kvp.Value;
+
+                    if (sendTime < now)
                     {
-                        Util.FireAndForget(delegate(object o) { SaveAppearance(kvp.Key); });
-                        m_savequeue.Remove(kvp.Key);
+                        Util.FireAndForget(o => SaveAppearance(avatarID));
+                        m_savequeue.Remove(avatarID);
                     }
                 }
             }
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 29966f9..3e3fb0f 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -2628,7 +2628,8 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         public void SendAppearanceToAllOtherAgents()
         {
-            //m_log.DebugFormat("[SCENE PRESENCE] SendAppearanceToAllOtherAgents: {0} ({1})", Name, UUID);
+//            m_log.DebugFormat("[SCENE PRESENCE] SendAppearanceToAllOtherAgents: {0} {1}", Name, UUID);
+
             // only send update from root agents to other clients; children are only "listening posts"
             if (IsChildAgent)
             {
@@ -2656,7 +2657,7 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         public void SendOtherAgentsAppearanceToMe()
         {
-            //m_log.DebugFormat("[SCENE PRESENCE] SendOtherAgentsAppearanceToMe: {0} ({1})", Name, UUID);
+//            m_log.DebugFormat("[SCENE PRESENCE] SendOtherAgentsAppearanceToMe: {0} {1}", Name, UUID);
 
             int count = 0;
             m_scene.ForEachRootScenePresence(delegate(ScenePresence scenePresence)
-- 
cgit v1.1