search

tanguyantoine / react-native-music-control

Display and manage media controls on lock screen and notification center for iOS and Android.
https://www.npmjs.com/package/react-native-music-control
MIT License
696 stars 265 forks source link

Android: setNowPlaying is not thread safe #297

Open tothvoj-gl opened 5 years ago

tothvoj-gl commented 5 years ago

Description

This issue reopens this one: https://github.com/tanguyantoine/react-native-music-control/issues/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: https://github.com/square/okhttp/issues/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 commented 5 years ago

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

tothvoj-gl commented 5 years ago

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 commented 5 years ago

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

tanguyantoine commented 4 years ago

reproducible with 0.10.8?

vjhameed commented 4 years ago

anu update on this ?

tothvoj-gl commented 4 years ago

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 commented 4 years ago

Going to reopen this. PR's welcome!

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

tothvoj-gl commented 4 years ago

@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.