> If you call GetEnumerator() on a SyncStack instance, it's returning an
> Enumerator that is synchronized (on itself at the moment i think,
> but when change it to use a reference to the stack,
> it'll synchronize on the stack).
Yes, and that's wrong ;-)
Consider the following snippet. It assumes that methods
of the Enumerator are syncronized on the reference
to the parent stack:
Stack syncStack=Synchronized(stack);
// the calls to e are synched on its parent.
IEnumerator e=syncStack.GetEnumerator();
while (e.MoveNext()) {
DoSomethingWith(e.Current);
}
Although it protects Enumerator's internal state during each call, it
doesn't protects the collection from being concurrently modified between
calls.
That is, if you creating synchronized stack, you suppose it will be used
concurrently. What you really want is to lock it while you're iterating
through it. Or else you should know that there is no other thread accesing
it in between MoveNext() and Current. Or concurrency is simply not an issue
at the moment, so you don't need synchronization at all, and there is really
no difference between synched and unsynched Enumerators.
The point is that to protect the stack during enumeration the following code
should be used:
Stack syncStack=Synchronized(stack);
lock (syncStack) {
// the calls to e are not synched at all
IEnumerator e=syncStack.GetEnumerator();
while (e.MoveNext()) {
DoSomethingWith(e.Current);
}
}
That is, client code should manually lock the entire loop to ensure the
Stack integrity.
> At the very least we should be sure all the classes
> that are there are consistent.
Exactly! I also agree that it's probably too early to do something about it
right now, but maybe just start thinking about it.
--
Sergey