[Opensim-dev] Some advice

[Stefan Andersson]

Some generic advice for when dealing with collections:
 
#1 The best thing to do is to isolate exactly what operations the modules are supposed to be doing on the collection, and provide those operations more directly (perhaps thru a system facade or by supplying those methods on the owner of the dictionary)
 
#2 Failing that, I really really suggest we expose a public accessor that returns a Entity[] instead of the list directly. Even if this is costly (which is why you want 1) in the first place) it does guarantee that nobody messes with the dictionary in unintended ways.
#3 It's really dangerous (no, really!) to expose a collection ref publicly, as you will get spurious 'collection changed' exceptions all over the place. Remember that everybody has to honour the EXACT SAME lock root object for locking to even work.
 
#4 If you're hell-bent on exposing it, implement a thread-safe wrapper around the collection. This is what I use to call a 'manager'. Actually, while you're at it, do it so the 'Manager' implements #1 and you're good.
 
Now, in refactoring for #1, I usually 'lean on the compiler' and just introduce an empty class, like this
 
public List<string> Strings;
 
Becomes
 
protected StringManager m_stringManager;
public StringManager Strings 
{
get
{
return m_stringManager;
}
}
 
Now, when I compile, I get errors on all invocations on 'Strings', so I get to re-think why that old place needed the collection anyway, and create a more suitable (and most often more efficent) method on my new class StringManager for doing that.
 
When all errors are gone, you have your thread-safe, customized, encapsulated solution. Oh joy!
 
Voila,
/Stefan



> Date: Tue, 11 Dec 2007 16:06:13 +0800> From: adam at gwala.net> To: opensim-dev at lists.berlios.de> Subject: Re: [Opensim-dev] Nasty exception in Scene.cs> > Modules & Plugins will need access to this.> > Maybe we can make a self-locking property to replace it.> > Rename Entities to m_entities, then make:> > List<Entity> Entities {> get {> lock(m_entities) {> return m_entities;> }> }> }> > Regards,> > Adam> > Sean Dague wrote:> > I got a nasty exception in Scene.cs during the Update() call as follows:> > > > [SCENE] [12-10 04:02:11] Failed with exception> > System.InvalidOperationException: out of sync> > at> > System.Collections.Generic.Dictionary`2+Enumerator[libsecondlife.LLUUID,OpenSim.Region.Environment.Scenes.EntityBase].VerifyState> > () [0x00000] > > at> > System.Collections.Generic.Dictionary`2+Enumerator[libsecondlife.LLUUID,OpenSim.Region.Environment.Scenes.EntityBase].CurrentSlot> > () [0x00000] > > at> > System.Collections.Generic.Dictionary`2+Enumerator[libsecondlife.LLUUID,OpenSim.Region.Environment.Scenes.EntityBase].get_Current> > () [0x00000] > > at> > System.Collections.Generic.Dictionary`2+ValueCollection+Enumerator[libsecondlife.LLUUID,OpenSim.Region.Environment.Scenes.EntityBase].get_Current> > () [0x00000] > > at> > System.Collections.Generic.Dictionary`2+ValueCollection[libsecondlife.LLUUID,OpenSim.Region.Environment.Scenes.EntityBase].CopyTo> > (OpenSim.Region.Environment.Scenes.EntityBase[] , Int32 ) [0x00000] > > at> > System.Collections.Generic.List`1[OpenSim.Region.Environment.Scenes.EntityBase].AddCollection> > (ICollection`1 ) [0x00000] > > at> > System.Collections.Generic.List`1[OpenSim.Region.Environment.Scenes.EntityBase]..ctor> > (IEnumerable`1 ) [0x00000] > > at OpenSim.Region.Environment.Scenes.InnerScene.UpdateEntities ()> > [0x00000] > > at OpenSim.Region.Environment.Scenes.Scene.Update () [0x00000] > > > > Upon further inspection, the issue here appears to be that we give> > public access to m_InnerScene.Entities, which is a dictionary, which is> > something that we need to be able to lock and iterate over inside the> > Scene. This problem can't be fixed with locks.> > > > Who needs direct access to that entities list outside of Scene? Is> > there some safer interface we can give them for what they really need?> > > > -Sean> > > > > > > > ------------------------------------------------------------------------> > > > _______________________________________________> > Opensim-dev mailing list> > Opensim-dev at lists.berlios.de> > https://lists.berlios.de/mailman/listinfo/opensim-dev> > _______________________________________________> Opensim-dev mailing list> Opensim-dev at lists.berlios.de> https://lists.berlios.de/mailman/listinfo/opensim-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://opensimulator.org/pipermail/opensim-dev/attachments/20071212/eaacb385/attachment-0001.html>