MarathonLabs / marathon

Cross-platform test runner
https://docs.marathonlabs.io
GNU General Public License v2.0
584 stars 122 forks source link

ConcurrentModificationException in LogcatManager #882

Closed VictorIreri closed 10 months ago

VictorIreri commented 10 months ago

Describe the bug I saw exception in the logs today for the first time when running tests on physical android devices. The test execution was able to continue after a small delay so it may have happened before and I didn't notice. I don't have much information about how to reproduce this unfortunately but hopefully the details below will be useful.

Expected behaviour Exception shouldn't occur.

Logs and reports

Exception in thread "LogcatManager-3 @coroutine#119" java.util.ConcurrentModificationException
    at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1043)
    at java.base/java.util.ArrayList$Itr.next(ArrayList.java:997)
    at com.malinskiy.marathon.android.adam.AdamAndroidDevice.onLine(AdamAndroidDevice.kt:527)
    at com.malinskiy.marathon.android.adam.LogcatManager$subscribe$1$1.invokeSuspend(LogcatManager.kt:38)
    at _COROUTINE._BOUNDARY._(CoroutineDebugging.kt:46)
    at com.malinskiy.marathon.android.adam.LogcatManager$subscribe$1.invokeSuspend(LogcatManager.kt:27)
    Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [CoroutineId(119), "coroutine#119":StandaloneCoroutine{Cancelling}@3dd9dee0, java.util.concurrent.ScheduledThreadPoolExecutor@49c409da[Running, pool size = 4, active threads = 1, queued tasks = 0, completed tasks = 308665]]
Caused by: java.util.ConcurrentModificationException
    at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1043)
    at java.base/java.util.ArrayList$Itr.next(ArrayList.java:997)
    at com.malinskiy.marathon.android.adam.AdamAndroidDevice.onLine(AdamAndroidDevice.kt:527)
    at com.malinskiy.marathon.android.adam.LogcatManager$subscribe$1$1.invokeSuspend(LogcatManager.kt:38)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:834)

Devices (please complete the following information):

Malinskiy commented 10 months ago

Hey @VictorIreri Currently, the access to log listeners is synchronized for add and remove, but not for traversal. I see two options: either synchronize the traversal or use some implementation of List that is thread-safe for our low-modification and high-traversal case. WDYT?

VictorIreri commented 10 months ago

Sorry I missed your response. Thanks for the fix.

Malinskiy commented 10 months ago

All good mate. Cheers