Skip to content

Fix ConcurrentModificationException in MultiConnectionKeeper#3553

Open
DaVinci9196 wants to merge 1 commit into
microg:masterfrom
DaVinci9196:fix_concurrentModificationException
Open

Fix ConcurrentModificationException in MultiConnectionKeeper#3553
DaVinci9196 wants to merge 1 commit into
microg:masterfrom
DaVinci9196:fix_concurrentModificationException

Conversation

@DaVinci9196

Copy link
Copy Markdown
Contributor

06-10 11:38:00.115 E/AndroidRuntime(28150): FATAL EXCEPTION: main
06-10 11:38:00.115 E/AndroidRuntime(28150): Process: com.google.android.gms, PID: 28150
06-10 11:38:00.115 E/AndroidRuntime(28150): java.util.ConcurrentModificationException
06-10 11:38:00.115 E/AndroidRuntime(28150): at java.util.HashMap$HashIterator.nextNode(HashMap.java:1454)
06-10 11:38:00.115 E/AndroidRuntime(28150): at java.util.HashMap$KeyIterator.next(HashMap.java:1478)
06-10 11:38:00.115 E/AndroidRuntime(28150): at org.microg.gms.common.MultiConnectionKeeper$Connection$1.onServiceConnected(MultiConnectionKeeper.java:216)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.app.LoadedApk$ServiceDispatcher.doConnected(LoadedApk.java:2340)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.app.LoadedApk$ServiceDispatcher$RunConnection.run(LoadedApk.java:2373)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.os.Handler.handleCallback(Handler.java:966)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.os.Handler.dispatchMessage(Handler.java:110)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.os.Looper.loopOnce(Looper.java:205)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.os.Looper.loop(Looper.java:293)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.app.ActivityThread.processInnerLoop(ActivityThread.java:10272)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.app.ActivityThread.loopProcess(ActivityThread.java:10264)
06-10 11:38:00.115 E/AndroidRuntime(28150): at android.app.ActivityThread.main(ActivityThread.java:10255)
06-10 11:38:00.115 E/AndroidRuntime(28150): at java.lang.reflect.Method.invoke(Native Method)
06-10 11:38:00.115 E/AndroidRuntime(28150): at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
06-10 11:38:00.115 E/AndroidRuntime(28150): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1366)

@sysxda

sysxda commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

getting simiral error

06-07 16:35:22.191  4328  4328 D AndroidRuntime: Shutting down VM
06-07 16:35:22.194  4328  4328 E AndroidRuntime: FATAL EXCEPTION: main
06-07 16:35:22.194  4328  4328 E AndroidRuntime: Process: com.google.android.gms, PID: 4328
06-07 16:35:22.194  4328  4328 E AndroidRuntime: java.util.ConcurrentModificationException
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at java.util.HashMap$HashIterator.nextNode(HashMap.java:1603)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at java.util.HashMap$ValueIterator.next(HashMap.java:1631)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at org.microg.gms.auth.phone.SmsRetrieverCore.onDestroy(SmsRetrieverCore.kt:237)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.DefaultLifecycleObserverAdapter.onStateChanged(DefaultLifecycleObserverAdapter.kt:29)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.kt:322)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.LifecycleRegistry.backwardPass(LifecycleRegistry.kt:273)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.kt:290)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.kt:143)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.kt:126)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at androidx.lifecycle.ServiceLifecycleDispatcher$DispatchRunnable.run(ServiceLifecycleDispatcher.kt:92)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:1070)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:125)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at android.os.Looper.dispatchMessage(Looper.java:333)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at android.os.Looper.loopOnce(Looper.java:263)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:367)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:9345)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:569)
06-07 16:35:22.194  4328  4328 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:837)

@peterhel

Copy link
Copy Markdown

Nice catch — real bug, and the fix is correct. Ran a quick 5-whys to make sure it hits root, not just symptom:

  1. Why the crash? ConcurrentModificationException thrown while iterating connectionForwards in the serviceConnection callback.
  2. Why the CME? The HashSet was structurally modified mid-iteration from another thread.
  3. Why concurrent modification? The callbacks (onServiceConnected/onServiceDisconnected, ~lines 216/228) iterate on the main thread off-lock, while add/removeConnectionForward mutate the set under the synchronized bind/unbind path — read and write don't share a lock.
  4. Why no shared lock? The async ServiceConnection callbacks were never brought under the keeper's synchronization, nor was the set made thread-safe — the class assumed single-threaded access that the async-callback model breaks.
  5. Why missed? (root) Connection straddles two concurrency domains — caller-driven mutation (synchronized) and framework-driven async callbacks (unsynchronized) — over shared mutable state, with no single explicit thread-safety policy for it.

This PR closes #1#3 cleanly and hardens the shared fields with the concurrent set + volatile — the crash is gone, and it's the right scoped fix. The only root-level leftover is #5: a couple of compound check-then-act sequences (e.g. addConnectionForward does add(...) then if (connected) → onServiceConnected(...)) are still technically racy against the async callback that flips connected and iterates, so a forward could in theory double- or miss a callback. Latent, rare, and clearly out of scope here — just flagging for a possible follow-up.

Builds cleanly in-tree for me. LGTM 👍

(Tiny nit, take it or leave it: stray double space in connectionForwards = Collections....)

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.

3 participants