Hi everyone,
I just received a tip here from Simon Linden about a patch that we might
want to include in Snowglobe. It's for an internally filed bug
involving region-crossing problems, where attachments can, under rare
conditions, become unattached during teleport or region crossing. The
cause of the bug is a race condition:
* Avatar A has a LocalID of 2 on region A
* Avatar A's attachment has LocalID 3
* Avatar A moves onto region B
* Region B sends a full update message, changing the local ID of the
avatar to 3
* Viewer sees the changing LocalID for the avatar, erases the old entry
for 2, and adds a new one for 3
* Viewer sees the changing ID for the attachment, erases the entry for
3, and adds one for 4
In this patch, the 'remove from local id table' code was modified to
detect the above and not delete the entry. With the current bug, the
item should remain in inventory, but the viewer would just get stupid
about what state the item is in.
The repro is pretty tough if you don't have access to the sim code, so
it's a tough one to catch in the wild.
This writeup is done through a cursory scan of the commit logs and
internal JIRA discussion, and I could have a detail or two wrong, but
that's the basic gist.
There's a lot of extraneous commented code in this patch, which is
otherwise pretty small. This seems like an interesting candidate for
Snowglobe 1.1, but it does introduce some risk. Thoughts? Should we
try to cram this in 1.1, or hold off for 1.2?
I've filed an external counterpart for this issue here:
http://jira.secondlife.com/browse/VWR-14905
Rob
--- linden/branches/giab-viewer/giab-viewer-2/indra/newview/llviewerobjectlist.cpp 2009/06/06 01:18:40 123211
+++ linden/branches/giab-viewer/giab-viewer-2/indra/newview/llviewerobjectlist.cpp 2009/06/06 07:33:28 123212
@@ -171,11 +171,28 @@
U32 port = region_host.getPort();
U64 ipport = (((U64)ip) << 32) | (U64)port;
U32 index = sIPAndPortToIndex[ipport];
-
+
+ // llinfos << "Removing object from table, local ID " << local_id << ", ip " << ip << ":" << port << llendl;
+
U64 indexid = (((U64)index) << 32) | (U64)local_id;
- return sIndexAndLocalIDToUUID.erase(indexid) > 0 ? TRUE : FALSE;
+
+ std::map<U64, LLUUID>::iterator iter = sIndexAndLocalIDToUUID.find(indexid);
+ if (iter == sIndexAndLocalIDToUUID.end())
+ {
+ return FALSE;
+ }
+
+ // Found existing entry
+ if (iter->second == object.getID())
+ { // Full UUIDs match, so remove the entry
+ sIndexAndLocalIDToUUID.erase(iter);
+ return TRUE;
+ }
+ // UUIDs did not match - this would zap a valid entry, so don't erase it
+ //llinfos << "Tried to erase entry where id in table ("
+ // << iter->second << ") did not match object " << object.getID() << llendl;
}
-
+
return FALSE ;
}
@@ -197,6 +214,9 @@
U64 indexid = (((U64)index) << 32) | (U64)local_id;
sIndexAndLocalIDToUUID[indexid] = id;
+
+ //llinfos << "Adding object to table, full ID " << id
+ // << ", local ID " << local_id << ", ip " << ip << ":" << port << llendl;
}
S32 gFullObjectUpdates = 0;
@@ -242,8 +262,8 @@
{
if ( LLToolMgr::getInstance()->getCurrentTool() != LLToolPie::getInstance() )
{
- //llinfos << "DEBUG selecting " << objectp->mID << " "
- // << objectp->mLocalID << llendl;
+ // llinfos << "DEBUG selecting " << objectp->mID << " "
+ // << objectp->mLocalID << llendl;
LLSelectMgr::getInstance()->selectObjectAndFamily(objectp);
dialog_refresh_all();
}
@@ -287,7 +307,7 @@
{
size = mesgsys->getReceiveSize();
}
-// llinfos << "Received terse " << num_objects << " in " << size << " byte (" << size/num_objects << ")" << llendl;
+ // llinfos << "Received terse " << num_objects << " in " << size << " byte (" << size/num_objects << ")" << llendl;
}
else
{
@@ -301,7 +321,7 @@
size = mesgsys->getReceiveSize();
}
-// llinfos << "Received " << num_objects << " in " << size << " byte (" << size/num_objects << ")" << llendl;
+ // llinfos << "Received " << num_objects << " in " << size << " byte (" << size/num_objects << ")" << llendl;
gFullObjectUpdates += num_objects;
}
@@ -311,7 +331,7 @@
if (!regionp)
{
- llwarns << "Object update from unknown region!" << llendl;
+ llwarns << "Object update from unknown region! " << region_handle << llendl;
return;
}
@@ -350,7 +370,6 @@
U8 compbuffer[2048];
S32 uncompressed_length = 2048;
S32 compressed_length;
-
compressed_dp.reset();
U32 flags = 0;
@@ -391,7 +410,7 @@
gMessageSystem->getSenderPort());
if (fullid.isNull())
{
- //llwarns << "update for unknown localid " << local_id << " host " << gMessageSystem->getSender() << llendl;
+ // llwarns << "update for unknown localid " << local_id << " host " << gMessageSystem->getSender() << ":" << gMessageSystem->getSenderPort() << llendl;
mNumUnknownUpdates++;
}
}
@@ -405,7 +424,7 @@
gMessageSystem->getSenderPort());
if (fullid.isNull())
{
- //llwarns << "update for unknown localid " << local_id << " host " << gMessageSystem->getSender() << llendl;
+ // llwarns << "update for unknown localid " << local_id << " host " << gMessageSystem->getSender() << llendl;
mNumUnknownUpdates++;
}
}
@@ -413,19 +432,43 @@
{
mesgsys->getUUIDFast(_PREHASH_ObjectData, _PREHASH_FullID, fullid, i);
mesgsys->getU32Fast(_PREHASH_ObjectData, _PREHASH_ID, local_id, i);
- // llinfos << "Full Update, obj " << local_id << ", global ID" << fullid << "from " << mesgsys->getSender() << llendl;
+ // llinfos << "Full Update, obj " << local_id << ", global ID" << fullid << "from " << mesgsys->getSender() << llendl;
}
objectp = findObject(fullid);
// This looks like it will break if the local_id of the object doesn't change
// upon boundary crossing, but we check for region id matching later...
- if (objectp && (objectp->mLocalID != local_id))
- {
+ // Reset object local id and region pointer if things have changed
+ if (objectp &&
+ ((objectp->mLocalID != local_id) ||
+ (objectp->getRegion() != regionp)))
+ {
+ //if (objectp->getRegion())
+ //{
+ // llinfos << "Local ID change: Removing object from table, local ID " << objectp->mLocalID
+ // << ", id from message " << local_id << ", from "
+ // << LLHost(objectp->getRegion()->getHost().getAddress(), objectp->getRegion()->getHost().getPort())
+ // << ", full id " << fullid
+ // << ", objects id " << objectp->getID()
+ // << ", regionp " << (U32) regionp << ", object region " << (U32) objectp->getRegion()
+ // << llendl;
+ //}
removeFromLocalIDTable(*objectp);
setUUIDAndLocal(fullid,
local_id,
gMessageSystem->getSenderIP(),
gMessageSystem->getSenderPort());
+
+ if (objectp->mLocalID != local_id)
+ { // Update local ID in object with the one sent from the region
+ objectp->mLocalID = local_id;
+ }
+
+ if (objectp->getRegion() != regionp)
+ { // Object changed region, so update it
+ objectp->setRegion(regionp);
+ objectp->updateRegion(regionp); // for LLVOAvatar
+ }
}
if (!objectp)
@@ -434,7 +477,7 @@
{
if (update_type == OUT_TERSE_IMPROVED)
{
- // llinfos << "terse update for an unknown object:" << fullid << llendl;
+ // llinfos << "terse update for an unknown object:" << fullid << llendl;
continue;
}
}
@@ -445,7 +488,7 @@
{
if (update_type != OUT_FULL)
{
-// llinfos << "terse update for an unknown object:" << fullid << llendl;
+ // llinfos << "terse update for an unknown object:" << fullid << llendl;
continue;
}
@@ -455,7 +498,7 @@
if (mDeadObjects.find(fullid) != mDeadObjects.end())
{
mNumDeadObjectUpdates++;
- //llinfos << "update for a dead object:" << fullid << llendl;
+ // llinfos << "update for a dead object:" << fullid << llendl;
continue;
}
#endif
@@ -468,20 +511,6 @@
justCreated = TRUE;
mNumNewObjects++;
}
- else
- {
- if (objectp->getRegion() != regionp)
- {
- // Object has changed region! Update lookup tables, set region pointer.
- removeFromLocalIDTable(*objectp);
- setUUIDAndLocal(fullid,
- local_id,
- gMessageSystem->getSenderIP(),
- gMessageSystem->getSenderPort());
- objectp->setRegion(regionp);
- }
- objectp->updateRegion(regionp); // for LLVOAvatar
- }
if (objectp->isDead())
@@ -797,6 +826,14 @@
// Remove from object map so noone can look it up.
mUUIDObjectMap.erase(objectp->mID);
+
+ //if (objectp->getRegion())
+ //{
+ // llinfos << "cleanupReferences removing object from table, local ID " << objectp->mLocalID << ", ip "
+ // << objectp->getRegion()->getHost().getAddress() << ":"
+ // << objectp->getRegion()->getHost().getPort() << llendl;
+ //}
+
removeFromLocalIDTable(*objectp);
if (objectp->onActiveList())
_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/SLDev
Please read the policies before posting to keep unmoderated posting privileges
