Skip to content

Fix Collection was modified crash in ManualMotionDispatcher when scheduling during Update#291

Open
Sahasrara wants to merge 1 commit into
annulusgames:mainfrom
Sahasrara:fix/manual-dispatcher-collection-modified
Open

Fix Collection was modified crash in ManualMotionDispatcher when scheduling during Update#291
Sahasrara wants to merge 1 commit into
annulusgames:mainfrom
Sahasrara:fix/manual-dispatcher-collection-modified

Conversation

@Sahasrara

Copy link
Copy Markdown

Problem

ManualMotionDispatcher.Update and Reset enumerate the runners dictionary directly:

foreach (var kv in runners)
    kv.Value.Update(time, time, time);

The only place runners is mutated is runners.Add(...) in GetOrCreateRunner, which runs lazily the first time a given (TValue, TOptions, TAdapter) motion type is scheduled on the dispatcher.

During runner.Update(...), motion callbacks fire (per-frame Bind actions, WithOnComplete, WithOnCancel, loop callbacks). If such a callback schedules the first motion of a not-yet-seen type through this dispatcher, GetOrCreateRunner adds a new entry to runners while the foreach is iterating it, throwing:

InvalidOperationException: Collection was modified; enumeration operation may not execute.
  System.Collections.Generic.Dictionary`2+Enumerator[TKey,TValue].MoveNext ()
  LitMotion.ManualMotionDispatcher.Update (System.Double deltaTime) at ManualMotionDispatcher.cs:82

It's intermittent because it only triggers the first time each new motion type is introduced from inside a callback during Update. Reset() has the same latent issue (a cancel callback can schedule a new-type motion).

Fix

Iterate a snapshot of runners.Values instead of the live dictionary, so a runner added by a callback mid-iteration no longer invalidates the enumeration.

The snapshot uses a reused List<IUpdateRunner> buffer, filled via AddRange(runners.Values) (which takes List's ICollection.CopyTo fast path; Dictionary caches its ValueCollection). So it stays allocation-free after warmup — no per-frame allocation is introduced. A runner created during Update simply ticks on the next frame.

Notes

  • No public API change.
  • Applied to both Update and Reset.

…ng during Update

ManualMotionDispatcher.Update and Reset enumerated the runners dictionary
directly. A motion callback invoked during iteration (Bind/OnComplete/
OnCancel/loop callbacks) can schedule the first motion of a new type, which
lazily adds a runner via GetOrCreateRunner and mutates the dictionary
mid-enumeration, throwing "InvalidOperationException: Collection was modified;
enumeration operation may not execute."

Iterate a reused buffer snapshot of runners.Values instead of the live
dictionary. The buffer is reused across calls and filled via AddRange's
ICollection.CopyTo fast path, so this remains allocation-free after warmup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant