Getter and update methods of IdlingRegistry are thread-unsafe

[rvprasad]

Description

Issue: If the getter methods are executed concurrently with the corresponding update methods, the HashSet constructor in the getter methods may fail due to concurrent modification of the sets underlying resources/loopers.

Cause: SynchronizedSet's iterator accesses the contents of the underlying set without proper synchronization.

Fix1: Getter methods could use something like Collections.addAll(result, resources.toArray(new Resource[0])); to ensure the access to the content of resources/loopers is synchronized and, hence, thread-safe.

Fix2: If IdlingRegistry is not intended for concurrent use, then such restriction should be documented.

Steps to Reproduce

This gist mimics the use of SynchronizedSet in IdlingRegistry and illustrates the issue.

Expected Results

No exceptions should occur.

Actual Results

ConcurrentModificationException is thrown.

AndroidX Test and Android OS Versions

All versions of AndroidX Test. (I believe) All versions of Android OS version.

Link to a public git repo demonstrating the problem:

https://gist.github.com/rvprasad/e573212b3f0247b5d4b42a5eeb2dce75

[TWiStErRob]

When do you need to change/query the idling registry concurrently? As far as I know using Espresso with standard components everything happens on the instrumentation or main thread and things have a defined ordering (one of the main goals of Espresso).

I have a feeling they'll ask you for a repro that uses Espresso, from first reading this might be an X/Y.

[rvprasad]

A colleague encountered this failure at work when trying to understand the flakiness of a test. So, the exact repro is hard; hence, the distilled gist. We do plan to look into the test further to figure out if we are incorrectly using Espresso.

That said, I think there are two possibilities for change.

1. IdlingRegistry is not intended to be used concurrently. If so, this intention should be documented to avoid such issues.
2. IdlingRegistry is intended to be used concurrently. If so, this issue is a real bug that needs to be fixed, which is rather trivial.

[adazh]

https://github.com/android/android-test/commit/2971079bc9923b8f3787299b074a3d0c0a407775 was checked in to fix this issue. Closing.