Fix lifecycle crash - Change backstack from ArrayDeque to LinkedBlockingDeque

[Bryan-Lamb]

Affected Version: Conductor 3.2.0

Context: In a typical Controller, we begin observing events from our ViewModels (using channels/flows) in the LifecycleListener.postAttach() callback which often result in a navigation call such as pushController() or setRoot().

In particular with setRoot() we've been encountering an interesting crash for quite some time and after investigation narrowed down a reproducible scenario to be:

1. Slow API call triggered, where a successful response results in setRoot() being called
2. While API call is inflight, the App is backgrounded
3. App is resumed, postAttach is triggered and calls setRoot() while the Backstack is still being iterated over by the LifecyleHandler

There appears to be an existing issue with similar circumstances

Exception java.util.ConcurrentModificationException:
  at java.util.ArrayDeque$DeqIterator.next (ArrayDeque.java:631)
  at com.bluelinelabs.conductor.RouterTransaction.controller (RouterTransaction.java)
  at com.bluelinelabs.conductor.internal.LifecycleHandler.onActivityStarted (LifecycleHandler.java)
  at android.app.Application.dispatchActivityStarted (Application.java:405)
  at android.app.Activity.dispatchActivityStarted (Activity.java:1406)
  at android.app.Activity.onStart (Activity.java:1922)
  at androidx.fragment.app.FragmentController.dispatchActivityCreated (FragmentController.java)
  at androidx.appcompat.app.AppCompatActivity.onStart (AppCompatActivity.java)
  at android.app.Instrumentation.callActivityOnStart (Instrumentation.java:1455)
  at android.app.Activity.performStart (Activity.java:8315)
  at android.app.ActivityThread.handleStartActivity (ActivityThread.java:4136)
  at android.app.servertransaction.TransactionExecutor.performLifecycleSequence (TransactionExecutor.java:221)
  at android.app.servertransaction.TransactionExecutor.cycleToPath (TransactionExecutor.java:201)
  at android.app.servertransaction.TransactionExecutor.executeLifecycleState (TransactionExecutor.java:173)
  at android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:97)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2443)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loopOnce (Looper.java:226)
  at android.os.Looper.loop (Looper.java:313)
  at android.app.ActivityThread.main (ActivityThread.java:8751)
  at java.lang.reflect.Method.invoke (Method.java)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:571)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1135)

Suggested Change Make Backstack synchronised by changing it's type from an ArrayDeque to a LinkedBlockingDeque

I've added a test to simulate this scenario which will fail with the previous ArrayDeque implementation, confirmed all tests are passing, but interested to know if there may be other unintended consequences of this change.

[RedaMohsen]

The same crash randomly happened to my app on Android 12 as reported by crashlytics. Hopefully this is a valid solution and the newest version will not have this issue.

[Bryan-Lamb]

Interested to hear any thoughts you may have on this @EricKuck

[daewin]

+1 We've observed this crash happening for a while now too

[EricKuck]

Thanks for taking a look at this one. I'm not sure this is the fix we need though. The test actually passes without any changes to the backstack. Additionally, is blocking the right way to handle this? If all changes are happening on the main thread, that could result in a deadlock.