From 06617ffd06c743cebffa768bc56a979f473b5b5b Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Fri, 22 Jun 2012 00:18:30 +0100
Subject: Add regression test for updating attachment position
---
.../Attachments/Tests/AttachmentsModuleTests.cs | 30 ++++++++++++++++++++++
1 file changed, 30 insertions(+)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 5e89eec..94c0030 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -303,6 +303,36 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
Assert.That(presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest));
}
+ [Test]
+ public void TestUpdateAttachmentPosition()
+ {
+ TestHelpers.InMethod();
+
+ UUID userId = TestHelpers.ParseTail(0x1);
+ UUID attItemId = TestHelpers.ParseTail(0x2);
+ UUID attAssetId = TestHelpers.ParseTail(0x3);
+ string attName = "att";
+
+ UserAccountHelpers.CreateUserWithInventory(scene, userId);
+ InventoryItemBase attItem
+ = UserInventoryHelpers.CreateInventoryItem(
+ scene, attName, attItemId, attAssetId, userId, InventoryType.Object);
+
+ AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+ acd.Appearance = new AvatarAppearance();
+ acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID);
+ ScenePresence sp = SceneHelpers.AddScenePresence(scene, acd);
+
+ SceneObjectGroup attSo = sp.GetAttachments()[0];
+
+ Vector3 newPosition = new Vector3(1, 2, 4);
+
+ scene.SceneGraph.UpdatePrimGroupPosition(attSo.LocalId, newPosition, sp.ControllingClient);
+
+ Assert.That(attSo.AbsolutePosition, Is.EqualTo(sp.AbsolutePosition));
+ Assert.That(attSo.RootPart.AttachedPos, Is.EqualTo(newPosition));
+ }
+
// I'm commenting this test because scene setup NEEDS InventoryService to
// be non-null
//[Test]
--
cgit v1.1
From 798846c5b6c05f37c661dde70fb9aaf51306e407 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Fri, 22 Jun 2012 00:39:41 +0100
Subject: refactor AttachmentsModule tests to use a common method for standard
attachment item setup
---
.../Attachments/Tests/AttachmentsModuleTests.cs | 139 ++++++++++-----------
1 file changed, 69 insertions(+), 70 deletions(-)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 94c0030..8f4a807 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -58,6 +58,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
private AttachmentsModule m_attMod;
private ScenePresence m_presence;
+ ///
+ /// Standard user ID
+ ///
+ private UUID m_userId = TestHelpers.ParseTail(0x1);
+
+// ///
+// /// Standard attachment item ID
+// ///
+// private UUID m_attItemId = TestHelpers.ParseTail(0x10);
+//
+// ///
+// /// Standard attachment asset ID
+// ///
+// private UUID m_attAssetId = TestHelpers.ParseTail(0x11);
+
[TestFixtureSetUp]
public void FixtureInit()
{
@@ -86,13 +101,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
}
///
- /// Add the standard presence for a test.
+ /// Creates an attachment item in the given user's inventory. Does not attach.
///
- private void AddPresence()
+ ///
+ /// A user with the given ID and an inventory must already exist.
+ ///
+ ///
+ /// The attachment item.
+ ///
+ ///
+ ///
+ ///
+ ///
+ private InventoryItemBase CreateAttachmentItem(UUID userId, string attName, int rawItemId, int rawAssetId)
{
- UUID userId = TestHelpers.ParseTail(0x1);
- UserAccountHelpers.CreateUserWithInventory(scene, userId);
- m_presence = SceneHelpers.AddScenePresence(scene, userId);
+ return UserInventoryHelpers.CreateInventoryItem(
+ scene,
+ attName,
+ TestHelpers.ParseTail(rawItemId),
+ TestHelpers.ParseTail(rawAssetId),
+ userId,
+ InventoryType.Object);
}
[Test]
@@ -101,7 +130,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
TestHelpers.InMethod();
// TestHelpers.EnableLogging();
- AddPresence();
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ m_presence = SceneHelpers.AddScenePresence(scene, m_userId);
+
string attName = "att";
SceneObjectGroup so = SceneHelpers.AddSceneObject(scene, attName, m_presence.UUID).ParentGroup;
@@ -140,24 +171,20 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
TestHelpers.InMethod();
// log4net.Config.XmlConfigurator.Configure();
- AddPresence();
-
- UUID attItemId = TestHelpers.ParseTail(0x2);
- UUID attAssetId = TestHelpers.ParseTail(0x3);
- string attName = "att";
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ m_presence = SceneHelpers.AddScenePresence(scene, m_userId);
- UserInventoryHelpers.CreateInventoryItem(
- scene, attName, attItemId, attAssetId, m_presence.UUID, InventoryType.Object);
+ InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20);
m_attMod.RezSingleAttachmentFromInventory(
- m_presence, attItemId, (uint)AttachmentPoint.Chest);
+ m_presence, attItem.ID, (uint)AttachmentPoint.Chest);
// Check scene presence status
Assert.That(m_presence.HasAttachments(), Is.True);
List attachments = m_presence.GetAttachments();
Assert.That(attachments.Count, Is.EqualTo(1));
SceneObjectGroup attSo = attachments[0];
- Assert.That(attSo.Name, Is.EqualTo(attName));
+ Assert.That(attSo.Name, Is.EqualTo(attItem.Name));
Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.Chest));
Assert.That(attSo.IsAttachment);
Assert.That(attSo.UsesPhysics, Is.False);
@@ -165,7 +192,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
// Check appearance status
Assert.That(m_presence.Appearance.GetAttachments().Count, Is.EqualTo(1));
- Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest));
+ Assert.That(m_presence.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo((int)AttachmentPoint.Chest));
}
[Test]
@@ -174,17 +201,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
TestHelpers.InMethod();
// log4net.Config.XmlConfigurator.Configure();
- AddPresence();
-
- UUID attItemId = TestHelpers.ParseTail(0x2);
- UUID attAssetId = TestHelpers.ParseTail(0x3);
- string attName = "att";
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ m_presence = SceneHelpers.AddScenePresence(scene, m_userId);
- UserInventoryHelpers.CreateInventoryItem(
- scene, attName, attItemId, attAssetId, m_presence.UUID, InventoryType.Object);
+ InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20);
- ISceneEntity so = m_attMod.RezSingleAttachmentFromInventory(
- m_presence, attItemId, (uint)AttachmentPoint.Chest);
+ ISceneEntity so
+ = m_attMod.RezSingleAttachmentFromInventory(
+ m_presence, attItem.ID, (uint)AttachmentPoint.Chest);
m_attMod.DetachSingleAttachmentToGround(m_presence, so.LocalId);
// Check scene presence status
@@ -196,7 +220,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
Assert.That(m_presence.Appearance.GetAttachments().Count, Is.EqualTo(0));
// Check item status
- Assert.That(scene.InventoryService.GetItem(new InventoryItemBase(attItemId)), Is.Null);
+ Assert.That(scene.InventoryService.GetItem(new InventoryItemBase(attItem.ID)), Is.Null);
// Check object in scene
Assert.That(scene.GetSceneObjectGroup("att"), Is.Not.Null);
@@ -208,18 +232,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
TestHelpers.InMethod();
// log4net.Config.XmlConfigurator.Configure();
- AddPresence();
-
- UUID attItemId = TestHelpers.ParseTail(0x2);
- UUID attAssetId = TestHelpers.ParseTail(0x3);
- string attName = "att";
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ m_presence = SceneHelpers.AddScenePresence(scene, m_userId);
- UserInventoryHelpers.CreateInventoryItem(
- scene, attName, attItemId, attAssetId, m_presence.UUID, InventoryType.Object);
+ InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20);
m_attMod.RezSingleAttachmentFromInventory(
- m_presence, attItemId, (uint)AttachmentPoint.Chest);
- m_attMod.DetachSingleAttachmentToInv(m_presence, attItemId);
+ m_presence, attItem.ID, (uint)AttachmentPoint.Chest);
+ m_attMod.DetachSingleAttachmentToInv(m_presence, attItem.ID);
// Check status on scene presence
Assert.That(m_presence.HasAttachments(), Is.False);
@@ -227,7 +247,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
Assert.That(attachments.Count, Is.EqualTo(0));
// Check item status
- Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo(0));
+ Assert.That(m_presence.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo(0));
}
///
@@ -239,17 +259,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
TestHelpers.InMethod();
// log4net.Config.XmlConfigurator.Configure();
- UUID userId = TestHelpers.ParseTail(0x1);
- UUID attItemId = TestHelpers.ParseTail(0x2);
- UUID attAssetId = TestHelpers.ParseTail(0x3);
- string attName = "att";
-
- UserAccountHelpers.CreateUserWithInventory(scene, userId);
- InventoryItemBase attItem
- = UserInventoryHelpers.CreateInventoryItem(
- scene, attName, attItemId, attAssetId, userId, InventoryType.Object);
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20);
- AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+ AgentCircuitData acd = SceneHelpers.GenerateAgentData(m_userId);
acd.Appearance = new AvatarAppearance();
acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID);
ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd);
@@ -268,17 +281,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
TestHelpers.InMethod();
// log4net.Config.XmlConfigurator.Configure();
- UUID userId = TestHelpers.ParseTail(0x1);
- UUID attItemId = TestHelpers.ParseTail(0x2);
- UUID attAssetId = TestHelpers.ParseTail(0x3);
- string attName = "att";
-
- UserAccountHelpers.CreateUserWithInventory(scene, userId);
- InventoryItemBase attItem
- = UserInventoryHelpers.CreateInventoryItem(
- scene, attName, attItemId, attAssetId, userId, InventoryType.Object);
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20);
- AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+ AgentCircuitData acd = SceneHelpers.GenerateAgentData(m_userId);
acd.Appearance = new AvatarAppearance();
acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID);
ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd);
@@ -288,7 +294,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
Assert.That(attachments.Count, Is.EqualTo(1));
SceneObjectGroup attSo = attachments[0];
- Assert.That(attSo.Name, Is.EqualTo(attName));
+ Assert.That(attSo.Name, Is.EqualTo(attItem.Name));
Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.Chest));
Assert.That(attSo.IsAttachment);
Assert.That(attSo.UsesPhysics, Is.False);
@@ -298,9 +304,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
List retreivedAttachments = presence.Appearance.GetAttachments();
Assert.That(retreivedAttachments.Count, Is.EqualTo(1));
Assert.That(retreivedAttachments[0].AttachPoint, Is.EqualTo((int)AttachmentPoint.Chest));
- Assert.That(retreivedAttachments[0].ItemID, Is.EqualTo(attItemId));
- Assert.That(retreivedAttachments[0].AssetID, Is.EqualTo(attAssetId));
- Assert.That(presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest));
+ Assert.That(retreivedAttachments[0].ItemID, Is.EqualTo(attItem.ID));
+ Assert.That(retreivedAttachments[0].AssetID, Is.EqualTo(attItem.AssetID));
+ Assert.That(presence.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo((int)AttachmentPoint.Chest));
}
[Test]
@@ -308,17 +314,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
{
TestHelpers.InMethod();
- UUID userId = TestHelpers.ParseTail(0x1);
- UUID attItemId = TestHelpers.ParseTail(0x2);
- UUID attAssetId = TestHelpers.ParseTail(0x3);
- string attName = "att";
-
- UserAccountHelpers.CreateUserWithInventory(scene, userId);
- InventoryItemBase attItem
- = UserInventoryHelpers.CreateInventoryItem(
- scene, attName, attItemId, attAssetId, userId, InventoryType.Object);
+ UserAccountHelpers.CreateUserWithInventory(scene, m_userId);
+ InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20);
- AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+ AgentCircuitData acd = SceneHelpers.GenerateAgentData(m_userId);
acd.Appearance = new AvatarAppearance();
acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID);
ScenePresence sp = SceneHelpers.AddScenePresence(scene, acd);
--
cgit v1.1
From 4cf49369b51c21a0eadd719eb46f53d207e1e5f7 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Fri, 22 Jun 2012 01:39:39 +0100
Subject: Stop flicking IsAttachment false and then true in
AttachmentsModule.UpdateAttachmentPosition() in order to avoid a hud update
race condition.
Previously, setting IsAttachment to false then true was necessary to serialize the updated attachment object information.
However, UpdateAttachmentPosition no longer does this update.
Whilst IsAttachment is set to false there is a race condition where the update thread can wrongly send hud object updates to clients that do not own the hud, resulting in screen artifacts.
---
.../Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs | 10 ----------
1 file changed, 10 deletions(-)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index b74c0ba..a2b95eb 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -408,17 +408,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
if (!Enabled)
return;
- // First we save the
- // attachment point information, then we update the relative
- // positioning. Then we have to mark the object as NOT an
- // attachment. This is necessary in order to correctly save
- // and retrieve GroupPosition information for the attachment.
- // Finally, we restore the object's attachment status.
- uint attachmentPoint = sog.AttachmentPoint;
sog.UpdateGroupPosition(pos);
- sog.IsAttachment = false;
- sog.AbsolutePosition = sog.RootPart.AttachedPos;
- sog.AttachmentPoint = attachmentPoint;
sog.HasGroupChanged = true;
}
--
cgit v1.1
From dca04c7b61abb7b7ea70299a192425ce3bd05937 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Fri, 22 Jun 2012 23:16:18 +0100
Subject: Avoid a race condition where an incoming request to a script external
URL can trigger an exception is the URL was being removed at the same time.
This involves three steps
1) Return gracefully in UrlModule.HttpRequestHandler() instead of throwing an exception when the url cannot be found in its index
2) Return true instead of false in HasEvents() if no matching request is found in the map. This call will only happen in the first place for raced requests.
3) Return a 404 in GetEvents() if the request is not in the index, rather than a blank 200 OK.
Many thanks to Tom Haines in http://opensimulator.org/mantis/view.php?id=6051 for doing some of the work on this.
---
.../CoreModules/Scripting/LSLHttp/UrlModule.cs | 108 ++++++++++++++-------
1 file changed, 73 insertions(+), 35 deletions(-)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
index 61afc76..5c05500 100644
--- a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
+++ b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
@@ -64,17 +64,25 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
public string uri;
}
+ ///
+ /// This module provides external URLs for in-world scripts.
+ ///
public class UrlModule : ISharedRegionModule, IUrlModule
{
private static readonly ILog m_log =
LogManager.GetLogger(
MethodBase.GetCurrentMethod().DeclaringType);
- private Dictionary m_RequestMap =
- new Dictionary();
+ ///
+ /// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the request ID
+ /// randomly generated when a request is received for this URL.
+ ///
+ private Dictionary m_RequestMap = new Dictionary();
- private Dictionary m_UrlMap =
- new Dictionary();
+ ///
+ /// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the full URL
+ ///
+ private Dictionary m_UrlMap = new Dictionary();
///
/// Maximum number of external urls that can be set up by this module.
@@ -224,7 +232,6 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
urlData.urlcode = urlcode;
urlData.requests = new Dictionary();
-
m_UrlMap[url] = urlData;
string uri = "/lslhttps/" + urlcode.ToString() + "/";
@@ -286,7 +293,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
{
if (m_RequestMap.ContainsKey(requestId))
{
- UrlData urlData=m_RequestMap[requestId];
+ UrlData urlData = m_RequestMap[requestId];
string value;
if (urlData.requests[requestId].headers.TryGetValue(header,out value))
return value;
@@ -295,6 +302,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
{
m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId);
}
+
return String.Empty;
}
@@ -339,6 +347,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
{
RemoveUrl(url.Value);
removeURLs.Add(url.Key);
+
foreach (UUID req in url.Value.requests.Keys)
m_RequestMap.Remove(req);
}
@@ -349,20 +358,31 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
}
}
-
private void RemoveUrl(UrlData data)
{
- m_HttpServer.RemoveHTTPHandler("", "/lslhttp/"+data.urlcode.ToString()+"/");
+ m_HttpServer.RemoveHTTPHandler("", "/lslhttp/" + data.urlcode.ToString() + "/");
}
private Hashtable NoEvents(UUID requestID, UUID sessionID)
{
Hashtable response = new Hashtable();
UrlData url;
+
lock (m_RequestMap)
{
+ // We need to return a 404 here in case the request URL was removed at exactly the same time that a
+ // request was made. In this case, the request thread can outrace llRemoveURL() and still be polling
+ // for the request ID.
if (!m_RequestMap.ContainsKey(requestID))
+ {
+ response["int_response_code"] = 404;
+ response["str_response_string"] = "";
+ response["keepalive"] = false;
+ response["reusecontext"] = false;
+
return response;
+ }
+
url = m_RequestMap[requestID];
}
@@ -384,53 +404,57 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
return response;
}
-
return response;
}
private bool HasEvents(UUID requestID, UUID sessionID)
{
- UrlData url=null;
+ UrlData url = null;
lock (m_RequestMap)
{
+ // We return true here because an external URL request that happened at the same time as an llRemoveURL()
+ // can still make it through to HttpRequestHandler(). That will return without setting up a request
+ // when it detects that the URL has been removed. The poller, however, will continue to ask for
+ // events for that request, so here we will signal that there are events and in GetEvents we will
+ // return a 404.
if (!m_RequestMap.ContainsKey(requestID))
{
- return false;
+ return true;
}
+
url = m_RequestMap[requestID];
if (!url.requests.ContainsKey(requestID))
{
- return false;
+ return true;
}
}
- if (System.Environment.TickCount-url.requests[requestID].startTime>25000)
+ // Trigger return of timeout response.
+ if (System.Environment.TickCount - url.requests[requestID].startTime > 25000)
{
return true;
}
- if (url.requests[requestID].requestDone)
- return true;
- else
- return false;
-
+ return url.requests[requestID].requestDone;
}
+
private Hashtable GetEvents(UUID requestID, UUID sessionID, string request)
{
- UrlData url = null;
+ UrlData url = null;
RequestData requestData = null;
lock (m_RequestMap)
{
if (!m_RequestMap.ContainsKey(requestID))
- return NoEvents(requestID,sessionID);
+ return NoEvents(requestID, sessionID);
+
url = m_RequestMap[requestID];
requestData = url.requests[requestID];
}
if (!requestData.requestDone)
- return NoEvents(requestID,sessionID);
+ return NoEvents(requestID, sessionID);
Hashtable response = new Hashtable();
@@ -443,6 +467,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
response["reusecontext"] = false;
return response;
}
+
//put response
response["int_response_code"] = requestData.responseCode;
response["str_response_string"] = requestData.responseBody;
@@ -459,6 +484,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
return response;
}
+
public void HttpRequestHandler(UUID requestID, Hashtable request)
{
lock (request)
@@ -483,11 +509,22 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
pathInfo = uri.Substring(pos3);
- UrlData url = null;
- if (!is_ssl)
- url = m_UrlMap["http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp];
- else
- url = m_UrlMap["https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp];
+ UrlData urlData = null;
+
+ lock (m_UrlMap)
+ {
+ string url;
+
+ if (is_ssl)
+ url = "https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp;
+ else
+ url = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp;
+
+ // Avoid a race - the request URL may have been released via llRequestUrl() whilst this
+ // request was being processed.
+ if (!m_UrlMap.TryGetValue(url, out urlData))
+ return;
+ }
//for llGetHttpHeader support we need to store original URI here
//to make x-path-info / x-query-string / x-script-url / x-remote-ip headers
@@ -520,11 +557,10 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
queryString = queryString + key + "=" + val + "&";
}
}
+
if (queryString.Length > 1)
queryString = queryString.Substring(0, queryString.Length - 1);
-
}
-
}
//if this machine is behind DNAT/port forwarding, currently this is being
@@ -532,26 +568,28 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
requestData.headers["x-remote-ip"] = requestData.headers["remote_addr"];
requestData.headers["x-path-info"] = pathInfo;
requestData.headers["x-query-string"] = queryString;
- requestData.headers["x-script-url"] = url.url;
+ requestData.headers["x-script-url"] = urlData.url;
//requestData.ev = new ManualResetEvent(false);
- lock (url.requests)
+ lock (urlData.requests)
{
- url.requests.Add(requestID, requestData);
+ urlData.requests.Add(requestID, requestData);
}
+
lock (m_RequestMap)
{
- //add to request map
- m_RequestMap.Add(requestID, url);
+ m_RequestMap.Add(requestID, urlData);
}
- url.engine.PostScriptEvent(url.itemID, "http_request", new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
+ urlData.engine.PostScriptEvent(
+ urlData.itemID,
+ "http_request",
+ new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
//send initial response?
// Hashtable response = new Hashtable();
return;
-
}
catch (Exception we)
{
--
cgit v1.1
From 78143769bfdf316bbec63e6232bf9be993eb078a Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Fri, 22 Jun 2012 23:49:52 +0100
Subject: Resolve various race conditions between accessing and removing
external script URLs by more consistently locking on m_UrlMap
---
.../CoreModules/Scripting/LSLHttp/UrlModule.cs | 280 +++++++++++----------
1 file changed, 148 insertions(+), 132 deletions(-)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
index 5c05500..05d54f0 100644
--- a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
+++ b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs
@@ -41,13 +41,39 @@ using OpenSim.Region.Framework.Scenes;
namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
{
+ ///
+ /// Data describing an external URL set up by a script.
+ ///
public class UrlData
{
+ ///
+ /// Scene object part hosting the script
+ ///
public UUID hostID;
+
+ ///
+ /// The item ID of the script that requested the URL.
+ ///
public UUID itemID;
+
+ ///
+ /// The script engine that runs the script.
+ ///
public IScriptModule engine;
+
+ ///
+ /// The generated URL.
+ ///
public string url;
+
+ ///
+ /// The random UUID component of the generated URL.
+ ///
public UUID urlcode;
+
+ ///
+ /// The external requests currently being processed or awaiting retrieval for this URL.
+ ///
public Dictionary requests;
}
@@ -77,6 +103,10 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
/// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the request ID
/// randomly generated when a request is received for this URL.
///
+ ///
+ /// Manipulation or retrieval from this dictionary must be locked on m_UrlMap to preserve consistency with
+ /// m_UrlMap
+ ///
private Dictionary m_RequestMap = new Dictionary();
///
@@ -113,10 +143,9 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
{
m_ExternalHostNameForLSL = config.Configs["Network"].GetString("ExternalHostNameForLSL", System.Environment.MachineName);
bool ssl_enabled = config.Configs["Network"].GetBoolean("https_listener",false);
+
if (ssl_enabled)
- {
https_port = (uint) config.Configs["Network"].GetInt("https_port",0);
- }
IConfig llFunctionsConfig = config.Configs["LL-Functions"];
@@ -275,32 +304,38 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
public void HttpResponse(UUID request, int status, string body)
{
- if (m_RequestMap.ContainsKey(request))
- {
- UrlData urlData = m_RequestMap[request];
- urlData.requests[request].responseCode = status;
- urlData.requests[request].responseBody = body;
- //urlData.requests[request].ev.Set();
- urlData.requests[request].requestDone =true;
- }
- else
+ lock (m_UrlMap)
{
- m_log.Info("[HttpRequestHandler] There is no http-in request with id " + request.ToString());
+ if (m_RequestMap.ContainsKey(request))
+ {
+ UrlData urlData = m_RequestMap[request];
+ urlData.requests[request].responseCode = status;
+ urlData.requests[request].responseBody = body;
+ //urlData.requests[request].ev.Set();
+ urlData.requests[request].requestDone =true;
+ }
+ else
+ {
+ m_log.Info("[HttpRequestHandler] There is no http-in request with id " + request.ToString());
+ }
}
}
public string GetHttpHeader(UUID requestId, string header)
{
- if (m_RequestMap.ContainsKey(requestId))
- {
- UrlData urlData = m_RequestMap[requestId];
- string value;
- if (urlData.requests[requestId].headers.TryGetValue(header,out value))
- return value;
- }
- else
+ lock (m_UrlMap)
{
- m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId);
+ if (m_RequestMap.ContainsKey(requestId))
+ {
+ UrlData urlData = m_RequestMap[requestId];
+ string value;
+ if (urlData.requests[requestId].headers.TryGetValue(header, out value))
+ return value;
+ }
+ else
+ {
+ m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId);
+ }
}
return String.Empty;
@@ -308,7 +343,8 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
public int GetFreeUrls()
{
- return m_TotalUrls - m_UrlMap.Count;
+ lock (m_UrlMap)
+ return m_TotalUrls - m_UrlMap.Count;
}
public void ScriptRemoved(UUID itemID)
@@ -366,9 +402,9 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
private Hashtable NoEvents(UUID requestID, UUID sessionID)
{
Hashtable response = new Hashtable();
- UrlData url;
+ UrlData urlData;
- lock (m_RequestMap)
+ lock (m_UrlMap)
{
// We need to return a 404 here in case the request URL was removed at exactly the same time that a
// request was made. In this case, the request thread can outrace llRemoveURL() and still be polling
@@ -383,25 +419,22 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
return response;
}
- url = m_RequestMap[requestID];
- }
-
- if (System.Environment.TickCount - url.requests[requestID].startTime > 25000)
- {
- response["int_response_code"] = 500;
- response["str_response_string"] = "Script timeout";
- response["content_type"] = "text/plain";
- response["keepalive"] = false;
- response["reusecontext"] = false;
+ urlData = m_RequestMap[requestID];
- //remove from map
- lock (url)
+ if (System.Environment.TickCount - urlData.requests[requestID].startTime > 25000)
{
- url.requests.Remove(requestID);
+ response["int_response_code"] = 500;
+ response["str_response_string"] = "Script timeout";
+ response["content_type"] = "text/plain";
+ response["keepalive"] = false;
+ response["reusecontext"] = false;
+
+ //remove from map
+ urlData.requests.Remove(requestID);
m_RequestMap.Remove(requestID);
- }
- return response;
+ return response;
+ }
}
return response;
@@ -409,9 +442,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
private bool HasEvents(UUID requestID, UUID sessionID)
{
- UrlData url = null;
-
- lock (m_RequestMap)
+ lock (m_UrlMap)
{
// We return true here because an external URL request that happened at the same time as an llRemoveURL()
// can still make it through to HttpRequestHandler(). That will return without setting up a request
@@ -423,61 +454,61 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
return true;
}
- url = m_RequestMap[requestID];
- if (!url.requests.ContainsKey(requestID))
+ UrlData urlData = m_RequestMap[requestID];
+
+ if (!urlData.requests.ContainsKey(requestID))
{
return true;
}
- }
- // Trigger return of timeout response.
- if (System.Environment.TickCount - url.requests[requestID].startTime > 25000)
- {
- return true;
- }
+ // Trigger return of timeout response.
+ if (System.Environment.TickCount - urlData.requests[requestID].startTime > 25000)
+ {
+ return true;
+ }
- return url.requests[requestID].requestDone;
+ return urlData.requests[requestID].requestDone;
+ }
}
private Hashtable GetEvents(UUID requestID, UUID sessionID, string request)
{
- UrlData url = null;
- RequestData requestData = null;
+ Hashtable response;
- lock (m_RequestMap)
+ lock (m_UrlMap)
{
+ UrlData url = null;
+ RequestData requestData = null;
+
if (!m_RequestMap.ContainsKey(requestID))
return NoEvents(requestID, sessionID);
url = m_RequestMap[requestID];
requestData = url.requests[requestID];
- }
- if (!requestData.requestDone)
- return NoEvents(requestID, sessionID);
-
- Hashtable response = new Hashtable();
+ if (!requestData.requestDone)
+ return NoEvents(requestID, sessionID);
- if (System.Environment.TickCount - requestData.startTime > 25000)
- {
- response["int_response_code"] = 500;
- response["str_response_string"] = "Script timeout";
+ response = new Hashtable();
+
+ if (System.Environment.TickCount - requestData.startTime > 25000)
+ {
+ response["int_response_code"] = 500;
+ response["str_response_string"] = "Script timeout";
+ response["content_type"] = "text/plain";
+ response["keepalive"] = false;
+ response["reusecontext"] = false;
+ return response;
+ }
+
+ //put response
+ response["int_response_code"] = requestData.responseCode;
+ response["str_response_string"] = requestData.responseBody;
response["content_type"] = "text/plain";
response["keepalive"] = false;
response["reusecontext"] = false;
- return response;
- }
- //put response
- response["int_response_code"] = requestData.responseCode;
- response["str_response_string"] = requestData.responseBody;
- response["content_type"] = "text/plain";
- response["keepalive"] = false;
- response["reusecontext"] = false;
-
- //remove from map
- lock (url)
- {
+ //remove from map
url.requests.Remove(requestID);
m_RequestMap.Remove(requestID);
}
@@ -487,44 +518,41 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
public void HttpRequestHandler(UUID requestID, Hashtable request)
{
- lock (request)
- {
- string uri = request["uri"].ToString();
- bool is_ssl = uri.Contains("lslhttps");
+ string uri = request["uri"].ToString();
+ bool is_ssl = uri.Contains("lslhttps");
- try
- {
- Hashtable headers = (Hashtable)request["headers"];
+ try
+ {
+ Hashtable headers = (Hashtable)request["headers"];
// string uri_full = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri;// "/lslhttp/" + urlcode.ToString() + "/";
- int pos1 = uri.IndexOf("/");// /lslhttp
- int pos2 = uri.IndexOf("/", pos1 + 1);// /lslhttp/
- int pos3 = uri.IndexOf("/", pos2 + 1);// /lslhttp//
- string uri_tmp = uri.Substring(0, pos3 + 1);
- //HTTP server code doesn't provide us with QueryStrings
- string pathInfo;
- string queryString;
- queryString = "";
+ int pos1 = uri.IndexOf("/");// /lslhttp
+ int pos2 = uri.IndexOf("/", pos1 + 1);// /lslhttp/
+ int pos3 = uri.IndexOf("/", pos2 + 1);// /lslhttp//
+ string uri_tmp = uri.Substring(0, pos3 + 1);
+ //HTTP server code doesn't provide us with QueryStrings
+ string pathInfo;
+ string queryString;
+ queryString = "";
- pathInfo = uri.Substring(pos3);
+ pathInfo = uri.Substring(pos3);
- UrlData urlData = null;
+ UrlData urlData = null;
- lock (m_UrlMap)
- {
- string url;
+ lock (m_UrlMap)
+ {
+ string url;
- if (is_ssl)
- url = "https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp;
- else
- url = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp;
+ if (is_ssl)
+ url = "https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp;
+ else
+ url = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp;
- // Avoid a race - the request URL may have been released via llRequestUrl() whilst this
- // request was being processed.
- if (!m_UrlMap.TryGetValue(url, out urlData))
- return;
- }
+ // Avoid a race - the request URL may have been released via llRequestUrl() whilst this
+ // request was being processed.
+ if (!m_UrlMap.TryGetValue(url, out urlData))
+ return;
//for llGetHttpHeader support we need to store original URI here
//to make x-path-info / x-query-string / x-script-url / x-remote-ip headers
@@ -544,6 +572,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
string value = (string)header.Value;
requestData.headers.Add(key, value);
}
+
foreach (DictionaryEntry de in request)
{
if (de.Key.ToString() == "querystringkeys")
@@ -570,34 +599,21 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
requestData.headers["x-query-string"] = queryString;
requestData.headers["x-script-url"] = urlData.url;
- //requestData.ev = new ManualResetEvent(false);
- lock (urlData.requests)
- {
- urlData.requests.Add(requestID, requestData);
- }
-
- lock (m_RequestMap)
- {
- m_RequestMap.Add(requestID, urlData);
- }
-
- urlData.engine.PostScriptEvent(
- urlData.itemID,
- "http_request",
- new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
-
- //send initial response?
-// Hashtable response = new Hashtable();
-
- return;
- }
- catch (Exception we)
- {
- //Hashtable response = new Hashtable();
- m_log.Warn("[HttpRequestHandler]: http-in request failed");
- m_log.Warn(we.Message);
- m_log.Warn(we.StackTrace);
+ urlData.requests.Add(requestID, requestData);
+ m_RequestMap.Add(requestID, urlData);
}
+
+ urlData.engine.PostScriptEvent(
+ urlData.itemID,
+ "http_request",
+ new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() });
+ }
+ catch (Exception we)
+ {
+ //Hashtable response = new Hashtable();
+ m_log.Warn("[HttpRequestHandler]: http-in request failed");
+ m_log.Warn(we.Message);
+ m_log.Warn(we.StackTrace);
}
}
@@ -606,4 +622,4 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp
ScriptRemoved(itemID);
}
}
-}
+}
\ No newline at end of file
--
cgit v1.1
From 5301648cff6b451fef4cca0baf8cda1bdb1455a6 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Mon, 25 Jun 2012 21:08:19 +0100
Subject: In AttachmentsModule.DetachSingleAttachmentToInvInternal(), remove
attachment before changing properties for correct inventory serialization.
Serialization of attachments requires IsAttachment = false so that correct positions are serialized instead of avatar position.
However, doing this when a hud is still attached allows race conditions with update threads, resulting in hud artifacts on other viewers.
This change sets SOG.IsDeleted before serialization changes take place (IsDeleted itself is not a serialized property).
LLClientView then screens out any deleted SOGs before sending updates to viewers.
---
OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index a2b95eb..99e0153 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -628,6 +628,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
{
m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
sp.RemoveAttachment(group);
+ m_scene.DeleteSceneObject(group, false);
// Prepare sog for storage
group.AttachedAvatar = UUID.Zero;
@@ -636,7 +637,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
group.AbsolutePosition = group.RootPart.AttachedPos;
UpdateKnownItem(sp, group, true);
- m_scene.DeleteSceneObject(group, false);
return;
}
--
cgit v1.1
From e5b739aaebace6b028f3f6bf05d21ff7a7c5affe Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Mon, 25 Jun 2012 22:48:13 +0100
Subject: When attachments are being saved and deleted for a closing root
agent, delete first to avoid a hud race condition with update threads.
If delete doesn't occur first then the update thread can outrace the IsAttachment = false necessary to save attachments and send hud artifacts to other viewers.
---
.../Avatar/Attachments/AttachmentsModule.cs | 33 ++++++++++++++--------
1 file changed, 21 insertions(+), 12 deletions(-)
(limited to 'OpenSim/Region/CoreModules')
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index 99e0153..2b0e4ab 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -152,31 +152,40 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
}
}
- public void SaveChangedAttachments(IScenePresence sp, bool saveAllScripted)
+ public void DeRezAttachments(IScenePresence sp, bool saveChanged, bool saveAllScripted)
{
-// m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
-
if (!Enabled)
return;
- foreach (SceneObjectGroup grp in sp.GetAttachments())
+// m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
+
+ lock (sp.AttachmentsSyncLock)
{
- grp.IsAttachment = false;
- grp.AbsolutePosition = grp.RootPart.AttachedPos;
- UpdateKnownItem(sp, grp, saveAllScripted);
- grp.IsAttachment = true;
+ foreach (SceneObjectGroup grp in sp.GetAttachments())
+ {
+ grp.Scene.DeleteSceneObject(grp, false);
+
+ if (saveChanged || saveAllScripted)
+ {
+ grp.IsAttachment = false;
+ grp.AbsolutePosition = grp.RootPart.AttachedPos;
+ UpdateKnownItem(sp, grp, saveAllScripted);
+ }
+ }
+
+ sp.ClearAttachments();
}
}
public void DeleteAttachmentsFromScene(IScenePresence sp, bool silent)
{
-// m_log.DebugFormat(
-// "[ATTACHMENTS MODULE]: Deleting attachments from scene {0} for {1}, silent = {2}",
-// m_scene.RegionInfo.RegionName, sp.Name, silent);
-
if (!Enabled)
return;
+// m_log.DebugFormat(
+// "[ATTACHMENTS MODULE]: Deleting attachments from scene {0} for {1}, silent = {2}",
+// m_scene.RegionInfo.RegionName, sp.Name, silent);
+
foreach (SceneObjectGroup sop in sp.GetAttachments())
{
sop.Scene.DeleteSceneObject(sop, silent);
--
cgit v1.1