Android: setNowPlaying is not thread safe

[tothvoj-gl]

Description

This issue reopens this one: #121
Quickly call setNowPlaying method multiple times. In real world, this use case happens e.g. when the user quickly presses next track button in the controller.

This issue is caused by unsafe concurrent access to a Buffer or other Okio stream: square/okhttp#3530

1. Stacktrace:
   java.lang.ArrayIndexOutOfBoundsException: size=603 offset=1467 byteCount=1         at com.android.okhttp.okio.Util.checkOffsetAndCount(Util.java:30)         at com.android.okhttp.okio.Buffer.getByte(Buffer.java:295)         at com.android.okhttp.okio.Buffer.readUtf8Line(Buffer.java:616)         at com.android.okhttp.okio.RealBufferedSource.readUtf8LineStrict(RealBufferedSource.java:203)         at com.android.okhttp.internal.http.Http1xStream.readResponse(Http1xStream.java:186)         at com.android.okhttp.internal.http.Http1xStream.readResponseHeaders(Http1xStream.java:127)         at com.android.okhttp.internal.http.HttpEngine.readNetworkResponse(HttpEngine.java:737)         at com.android.okhttp.internal.http.HttpEngine.readResponse(HttpEngine.java:609)         at com.android.okhttp.internal.huc.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:471)         at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:407)         at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getResponseCode(HttpURLConnectionImpl.java:538)         at com.android.okhttp.internal.huc.DelegatingHttpsURLConnection.getResponseCode(DelegatingHttpsURLConnection.java:105)         at com.android.okhttp.internal.huc.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:26)         at com.google.firebase.perf.network.zze.getInputStream(Unknown Source:56)         at com.google.firebase.perf.network.zzd.getInputStream(Unknown Source:11)         at com.tanguyantoine.react.MusicControlModule.loadArtwork(MusicControlModule.java:446)         at com.tanguyantoine.react.MusicControlModule.access$000(MusicControlModule.java:41)         at com.tanguyantoine.react.MusicControlModule$1.run(MusicControlModule.java:264)         at java.lang.Thread.run(Thread.java:764)

2. Platform ?

   • iOS
   • [x ] Android

3. Device

• [ x] Simulator
• [ x] Real device

[RWOverdijk]

@tothvoj-gl What would be the proper solution here?

[tothvoj-gl]

Typically ArrayIndexOutOfBoundsException in Okio indicates a thread-safety violation. If multiple threads are operating on the same Buffer at the same time, invariants are broken.

Unfortunately, segment pooling means that it is possible for a race condition in one area of the code to trigger a completely unrelated area of the code to fail. It’s pretty insidious.

Suppose we have three threads T1, T2, and T3, two buffers B1 and B2, and one segment S1.

T1 writes "foo" to B1, which creates S1.
T1 and T2 concurrently start a read 3 bytes from B1. They both read S1. This is a bug.
T1 reads the bytes, causing S1 to be put in the segment pool.
T3 writes "bar" to B2. which takes S1 from the segment pool.
Now T3 and T2 are both racing over S1, even though originally the race was between T1 and T2.

To avoid such races we need to avoid writing racy code!

It is a quote from the issue thread in Okio github.

[RWOverdijk]

I'll give this a go once I start using the module later this/next week.

[tanguyantoine]

reproducible with 0.10.8?

[vjhameed]

anu update on this ?

[tothvoj-gl]

We have switched to react-native-track-player, so cannot tell if this issue persists. But it was pretty easy to reproduce, just call setNowPlaying multiple times in a row.

[bradfloodx]

Going to reopen this. PR's welcome!

@tothvoj-gl How are you going with react-native-track-player?

[tothvoj-gl]

@bradleyflood We have less bugs after migrating to RN track-player and also the code is cleaner. However, we noticed some issues related to lock screen notification widgets, so if it is a primary feature for you, consider switching.