Skip to content

Race condition in ExtensionBase.ReproduceCohorts causes "Collection was modified; enumeration operation may not execute" with ThreadCount > 1 #25

Description

@Klemet

Issue written with some help from an LLM to make things clearer

Environment

  • LANDIS-II Core v8 (Linux)
  • Extension-PnET-Succession v6
  • Library-Succession (commit 538bf3c)
  • 4 threads (ThreadCount > 1), ~4,000,000 active sites

Description

Running a large-landscape scenario, the model crashed at timestep 60 during "Cohort reproduction":

Running PnET-Succession ...
growing cohorts ...
% done:   0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100%
          |----|----|----|----|----|----|----|----|----|----|
Progress: +++++++++++++++++++++++++++++++++++++++++++++++++++
Computing shade ...
% done:   0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100%
          |----|----|----|----|----|----|----|----|----|----|
Progress: +++++++++++++++++++++++++++++++++++++++++++++++++++
Cohort reproduction ...
% done:   0%  10%  20%  30%  40%  50%  60%  70%  80%  90%  100%
          |----|----|----|----|----|----|----|----|----|----|
Progress: ++++++++++++++++++++++++++++++++++++++++++++++++++Internal error occurred within the program:
  One or more errors occurred. (Collection was modified; enumeration operation may not execute.)
  Collection was modified; enumeration operation may not execute.

Stack trace:
   at System.Threading.Tasks.TaskReplicator.Run[TState](ReplicatableUserAction`1 action, ParallelOptions options, Boolean stopOnFirstFailure)
   at System.Threading.Tasks.Parallel.ForWorker[TLocal,TInt](TInt fromInclusive, TInt toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
--- End of stack trace from previous location ---
   at System.Threading.Tasks.Parallel.ForWorker[TLocal,TInt](TInt fromInclusive, TInt toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
   at System.Threading.Tasks.Parallel.For(Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body)
   at Landis.Library.Succession.ExtensionBase.ReproduceCohorts(IEnumerable`1 sites)
   at Landis.Library.Succession.ExtensionBase.Run()
   at Landis.Extension.Succession.BiomassPnET.PlugIn.Run() in /usr/bin/LANDIS_Linux/Core-Model-v8-LINUX/Extension-PnET-Succession/src/PlugIn.cs:line 697
   at Landis.Model.Run(String scenarioPath, IUserInterface ui)
   at Landis.App.Main(String[] args) in /usr/bin/LANDIS_Linux/Core-Model-v8-LINUX/Tool-Console/src/App.cs:line 98

Root cause

In ExtensionBase.ReproduceCohorts (src/ExtensionBase.cs, L328-356), when ThreadCount != 1, sites are processed with Parallel.For, one site per thread. The only synchronization (lock (threadLock)) protects the progress bar — nothing protects site data.

Inside Reproduction.Reproduce(site, randomGen) (src/Reproduction.cs), serotiny/resprout/planting call AddNewCohort(...), which mutates that site's cohort collection. When none of these apply, seeding.Do(site, randomGen) runs, which reads the cohort collections of neighboring sites to check for seed sources within the species' dispersal distance.

Because Parallel.For assigns different sites to different threads with no cross-site locking, thread A can be enumerating neighbor N's cohort list (seed search) at the same instant thread B — assigned to site N — is adding a new cohort to that same list (serotiny/resprout/planting). This is a classic concurrent read/modify race on a non-thread-safe collection (e.g. List<Cohort>), producing InvalidOperationException: Collection was modified; enumeration operation may not execute.

This explains why the crash is:

  • Non-deterministic — depends on thread scheduling, so timestep/seed may vary between runs.
  • Scale-dependent — probability grows with number of active sites and thread count, and spikes whenever many cells reproduce in the same timestep (e.g., after a disturbance causing wide-spread serotiny/resprouting), which is a plausible explanation for the crash appearing at timestep 60 rather than earlier.
  • Not hardware-related: it's a normal .NET BCL exception raised from Parallel.For/TaskReplicator, consistent with a software race, not memory/CPU corruption.

Impact

Any parallel-capable succession extension built on this library version (PnET, NECN, Biomass-Succession, etc.) is affected whenever ThreadCount > 1 and reproduction occurs.

Suggested fix

I'm not an expert, but I think that this could be fixed by implementing some "locks", just like Austen has added for the progress bar already. Locks prevent a thread from accessing a given object (e.g. the list of cohorts in a cell) if another thread is already modifying it.

I'm still working out where exactly in the code these need to be added. I'll try to test it by myself; if I'm not mistaken, if I use the same number of CPU cores/parallel threads in the simulation where I got this error, I should be able to trigger it again at the exact same moment. As such, I can use it as a test to see if I'm able to fix this. I'll propose a PR when/if I find the fix !

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions