Enumerator MoveNext - Multi-Threading Exception

Question

I guess the only way to explain this is to do so, and I hope I don't get too confusing. 

I have this Asynchronous DataLoader (i've mentioned it before) and each loader is attached to a DataTable.  (Typed or otherwise, it works both ways).  The loader is intuitive with regards to Parent Table Relations, and thus is designed to ensure constraints and relational integrity during the load process.  This is achieved through my "cover-all-bases" Asynchronous Worker class.  It's like this:

FlufCommand - Simple Command based object with abstract Execute(object) method, providing the framework for Progress and Completion notification events

FlufAsyncCommand (FlufCommand) - Extended Command object which adds the ExecuteAsync() method which uses an AsyncOperation object to post the progress and completion events (as well as any other events the descendant classes may wish to invoke) and a Thread to "run" the Inherited Abstract Execute(object) method.

FLufAsyncWorker(FlufAsyncCommand) - Final extended object (not sealed) which instead of requiring the override of the Execute() abstract method, it transforms that into an Event Call: PerformOperation() similar to the BackgroundWorker's DoWork event.  Additionally this class employs a "Lock Over Queue" stay-alive thread methodology similar to the ThreadPool.  What this does is allows an operation that is being performed to cycle back to to the top of the thread pool before releasing its locks if necessary.  (when combined with my data loader explanation this will make sense).

The Locks are handled as such: 
Every FlufAsyncCommand has:

•   A Single AutoResetEvent which is Reset at the start of the ThreadProc callback used by the Thread object, which allows the coder to listen for it elsewhere and set it inside their Execute() overridden method.
•   A queue of WaitHandles.  These WaitHandles are Passed to an ExecuteAsync() overload as an array of WaitHandles which are "waited" on at the start of the ThreadProc before the Execute() method is called, allowing the coder to start the async operation with a dynamicism of other operations that may or may not be needed.  IE: Other FlufAsyncCommands that could pass their (above) AutoResetEvent and thus this secondary command would wait untill the waithandles are in their Set() state.
•   A single ReaderWriterlockSlim.  This is the WriteLock for the owning FlufAsyncCOmmand object.  The WriteLock is always obtained before the Execute Method is invoked.
•   A Collection of ReaderWriterLockSlims - these are the ReadLocks for the FlufAsyncCOmmand and are typically provided WriteLocks from other AsyncCommands. 

Thus the ThreadProc() is executed as follows:

Trigger.Reset() 'AutoResetEvent
if WaitQueue.WaitAll() then 'WaitHandles provided.  if queue = 0 this returns true
   if ReadLocks.LockAll() andalso WriteLock.Lock() then 'These are actually wrappers around the read/write locks used avoid exceptions if a lock is already in locked state, so as not to nest locks unnecessarily. 
                     'Deduce that success/failure of the lock is the boolean result and will block the thread until lock is achieved
      Me.Result = Me.Execute(Argument)
   end if

Now, this is the cool part (in my mind at least, but I'm biased). 
The DataLoader examines its table at the start of the load process.  It determines if the table has any parent tables.  Now my dataloader uses some nifty interfaces to maintain communication between table and adapter, and thus the ParentTable executes a load based upon the Child Tables Page to Load.  so this happens like this;

(pseudo code)
Child.Load(pages() integer) {
  if (child.relations.count > 0) {
     for each rel in child.relations {
       rel.Table.ParentLoad(child, pages)
     }
  }
}

now the parentLoad basically sends to the Database with a Stored Procedure, the Child Table and The Pages of that child table, and using some other nifty SQL determines which pages (of say 1000 rows) in the parent table are required to be loaded so that the rows in the pages of the child table being loaded have their appropriate relational integrity in the Dataset. 

However, I need to assert that the Parent Pages are loaded first.  thus in the ParentLoad() method which i have conveniently passed the ChildLoader I do:

ParendLoad(ChildLoader, ChildPages) {
   pages() = GetParentPages(ChildLoader.Tablename, Pages)
   ChildLoader.AddReaderLock(Me.WriteLock)
   Load(Pages)
}

So, if this is the first execution of the the ParentTable/ChildTable loading sequence the ChildLoader's ReaderLocks Collection will be provided the WriteLock for the ParentTable.  And then before the ChildLoader can execute it's Load(Pages) method, the ParentTable starts it's process, which acquires all of its Reader locks and it's own WriterLock before execution.  If the CHildTable has 3 different Parent Tables, and just say maybe one of those ParentTables actually Shares a Different Parent Table with the Child Table:

Parent1
Parent2 - FK to Parent1
Parent3
Child1 - FK to Parent1, FK to Parent2, FK to Parent3

Child.Load() Follows this sequence now:
If you see this, the Parent1 is put in a WriteLock state and begun loading.
Parent2 Goes to Acquire its ReadLock, but it is dependent upon Parent1 so it is blocked.
Parent3 Acquires its Writelock and begins loading.
Child1 Goes to Acquire its Readlocks and finds that Parent1 is still blocking. 
Ah, but since readlocks can happen simultaneously in this circumstance, you can see a minor issue of the Parent2 Acquiring a readlock at the same time as child1, before it has loaded, due to a minor race condition that the child1 needs a read lock on parent2 but that could occur before parent2 has aquired its write lock.  This is where the LockOverQueue of the AsyncCOmmand comes in handle like the ThreadPool.   When the Load for Parent1 is Executed, it's loading rows that are directly required by relationship from Child1.  Child1 proceeds to setup the same situation with Parent2, but parent2 has a parent itself so it requests a different set of pages from parent1.  These are queued much like threadpool queues up tasks, and then is blocked by attempting to aquire readlock of parent1's writelock.  Instead of Parent1 releasing it's writelock at the end of the Execute() method, the loop determines if the ProcessQueue has another request waiting, (ie: parent2's related pages).  So, itdoes NOT release the locks, loops back around and runs the load() process again, this time on parent2's related pages, instead of child1's related pages.  then when the queue is empty, it releases it's writelock, and amazingly enough the parent2 writelock locks in, as it was attempting to acquire the read lock before child1, and child1 remains blocked until all three parents writelocks are released and then the child1 writelocks itself and loads its pages.

Very complicated, and yes we're dealing with thousands of lines of code that seems to work quite seamlessly as I intended, but i'm noticing a slight problem with the ReadLock collection enumerator. 

Currently, the AsyncCommand has the Collection of ReadLocks, and during the Load() operation confusingly described above, the ParentLoader adds it's WriteLock to the ChildLoader's ReadLock collection.  Somewhere during the multi-threaded race condition an invalid operation occurs with accessing the next element in the readlock collection.  below are the two sets of code involved:  A) How I add the ReadLock to the Collection, and B) how I block for all the ReaderLocks that it's trying to acquire:

   Public Function AddReaderLock(ByVal Name As String, ByVal ReaderLock As AsyncAccessLock) As Boolean
      If Name.isValid() AndAlso ReaderLock.isValid() Then
         If ReadLocks.HasKey(Name) Then
            ReadLocks(Name) = ReaderLock
         Else : ReadLocks.Add(Name, ReaderLock)
         End If
         Return ReaderLock.Equals(ReadLocks(Name))
      End If
      Return False
   End Function

   Public Function LockAll() As Boolean
      Dim b As Boolean = (Count = 0)
      If Not b Then
         b = True
         For Each lock As AsyncAccessLock In Me
            b = b AndAlso lock.LockRead() 
               'LockRead equates to:
               'If Not ReadLocked then readLocked = Lock.TryEnterReadLock(infinite)
         Next
      End If
      Return b
   End Function

My error is at the "Next" statement for the "For Each" loop in LockAll()

the error is:
InvalidOperationException
   Source: mscorlib
   Message: Collection was modified; enumeration operation may not execute.
   Stack:    at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Collections.Generic.List`1.Enumerator.MoveNext()
   at FlufLib.AccessLocksCollection.LockAll() in C:\Source\Visual Studio 2008\Projects\FlufLib\FlufLib\FlufAsyncWorker.vb:line 915
   at FlufLib.FlufAsyncCommand.ThreadProc(Object Argument) in C:\Source\Visual Studio 2008\Projects\FlufLib\FlufLib\FlufAsyncWorker.vb:line 215

So:
A) Do i need to synclock access to this collection, and how would I do it so as to prevent any form of lock-up.
B) Would simply moving away from the For-Each-Next into a While/Do loop resolve this exception?

Thanks
Jaeden "Sifo Dyas" al'Raec Ruiner
(PS - I know this was long for such a small question, but, when debugging threads its ALWAYS amusingly complicated.  *chuckle*)

---

"Never Trust a computer. Your brain is smarter than any micro-chip."

Answer 1

Make sure the lock As AsyncAccessLock object is not being modified inside of the loop.  Ditto for the collection.
The quantity of items cannot be modified, and you can get exceptions is you try to modify members.

If you want to modify the collection while you iterate over the entire collection, use a For/Next loop, instead of For/Each.

---

Mark the best replies as answers. "Fooling computers since 1971."

Answer 2

Ahh. 

yea.  I am not changing the Lock object or the AccessLockCollection inside that loop, however, it can be changed by a alternative thread.  that section is inside the worker thread, while the main ui thread could be prepping another table to load, no matter its dependencies, and thus the currently loading table could have another readlock added to the collection while iterating through that loop. 

I have changed it to a While (i < Count) /end while loop.  as I figured that While or Do loops are more dynamic to changes in the iteration count.  I sort of assumed (from my old pascal/c days) that the iterating for loop (using an index not the enumerating for each) might also be as static as the for-each loop is.  old Pascal made the "max" static at loop entry, thus for I:=0 to Count Do, meant that one could not change count, and if this was a collection and Count was a computed property the for loop did not recompute the Count value each iteration, thus it was necessary to use While or Repeat/Until.  (as the pascal syntax goes). 

This is good to know though, cause now in the process of fixing this, I have come across a wildly unanticipated thread glitch that I now have to resolve....but that's a question for a different thread.

Thanks
Jaeden "Sifo Dyas" al'Raec Ruiner

---

"Never Trust a computer. Your brain is smarter than any micro-chip."