Fix concurrent modification

[hiroshihorie]

Snapshot collections with .toList() / List.of() before iterating, preventing modification by concurrent async event handlers.

Summary by CodeRabbit

• Bug Fixes
  • Fixed potential concurrent modification exceptions that could occur during async operations when iterating over participant data, event listeners, and audio elements.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request fixes concurrent modification exceptions that occur when collections are mutated during iteration in asynchronous contexts. Multiple files are updated to create snapshots of collections using toList() before iterating, ensuring safe access during async operations and mutations.

Changes

Cohort / File(s)	Summary
Changelog .changes/fix-concurrent-modification	Added changelog entry documenting the concurrent modification fix.
Core Engine Components lib/src/core/room.dart, lib/src/core/signal_client.dart	Applied defensive copies via toList() to prevent concurrent modification during iteration in event handling, reconnection logic, and request queue processing.
Manager and Lifecycle lib/src/managers/event.dart, lib/src/e2ee/e2ee_manager.dart	Modified event listener cancellation and frame cryptor disposal to create collection snapshots before iteration and clearing to prevent modification during async operations.
Web Media lib/src/track/web/_audio_html.dart	Protected iteration over audio elements using toList() snapshot to prevent concurrent modification during playback.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hop through safety nets so fine,
Where iterations dance in perfect line,
Snapshots captured, copies made with care,
No concurrent woes to cause despair!
Async flows now safe and sound,
Bug-free collections, peace profound! 🎉

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'Fix concurrent modification' directly matches the main objective of the changeset, which is to fix concurrent modification issues across multiple files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch hiroshi/fix-concurrent-modification

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa7db38 and 1936bb5.

📒 Files selected for processing (6)

• .changes/fix-concurrent-modification
• lib/src/core/room.dart
• lib/src/core/signal_client.dart
• lib/src/e2ee/e2ee_manager.dart
• lib/src/managers/event.dart
• lib/src/track/web/_audio_html.dart

🔇 Additional comments (9)

lib/src/track/web/_audio_html.dart (1)

54-62: LGTM!

Snapshotting _audioElements.values with .toList() before the async loop correctly prevents concurrent modification if stopAudio mutates the map while await audio.play().toDart yields.

lib/src/e2ee/e2ee_manager.dart (2)

140-151: LGTM!

Snapshotting _frameCryptors.values before iterating with await in _cleanUp prevents concurrent modification if event handlers add/remove cryptors during disposal. The _frameCryptors.clear() after the loop is correctly placed.

---

179-211: LGTM!

Consistent use of .entries.toList() in setEnabled, setKeyIndex, and getKeyIndex — all of which await during iteration and could otherwise be affected by concurrent map mutations from event handlers.

lib/src/core/signal_client.dart (1)

516-525: LGTM!

The copy-then-clear pattern is a good defensive approach. By clearing _queue before iterating the copy, any re-entrant code that might add to _queue during dispatch won't interfere with the current batch, and newly enqueued requests won't be lost.

lib/src/managers/event.dart (1)

132-141: LGTM!

Copy-then-clear correctly prevents concurrent modification when await listener.cancel() yields. Listeners added during cancellation land safely in the freshly cleared _listeners and are not inadvertently cancelled.

lib/src/core/room.dart (3)

542-548: LGTM!

Snapshotting both _remoteParticipants and trackPublications.values before iterating in the EngineRestartedEvent handler prevents concurrent modification if participant/track state changes during rePublishAllTracks() or other interleaved async events.

---

939-948: LGTM!

Same defensive snapshot pattern in _sendSyncState — consistent with the rest of the PR.

---

1087-1091: LGTM!

Snapshot of _remoteParticipants in setAudioOutputDevice prevents modification during the iteration.

.changes/fix-concurrent-modification (1)

1-1: LGTM!

Changelog entry accurately describes the fix.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}