Allow removing a `Backstack.CompletionListener` from inside the listener itself

[angusholder]

Currently if you call Backstack.removeStateChangeCompletionListener() from within the completion listener, it throws a ConcurrentModificationException in NavigationCore's loop:

https://github.com/Zhuinden/simple-stack/blob/8d37157aee38ad54d3471b6e064ce9bb37ab86a1/simple-stack/src/main/java/com/zhuinden/simplestack/NavigationCore.java#L707-L711

I believe this could be solved by changing the above code to

for(Backstack.CompletionListener completionListener : new ArrayList(completionListeners)) {

Does that sound okay to change?

---

For context, my view controller is using addStateChangeCompletionListener to listen to enable NFC whenever it's in the foreground, and I remove the CompletionListener when my view controller's is destroyed (key is no longer in the backstack), like so:

    val backstackListener = object : Backstack.CompletionListener {
        override fun stateChangeCompleted(stateChange: StateChange) {
            if (controller.getKey() !in stateChange.getNewKeys<NavKey>()) {
                backstack.removeStateChangeCompletionListener(this) // Crash caused by this line
                NfcListeners.unregisterNfcCallback(nfcCallback)
                return
            }

            if (controller.getKey<NavKey>() == stateChange.topNewKey<NavKey>()) {
                // We're in the foreground, enable NFC
                NfcListeners.registerNfcCallback(nfcCallback)
            } else {
                NfcListeners.unregisterNfcCallback(nfcCallback)
            }
        }
    }
    backstack.addStateChangeCompletionListener(backstackListener)

I can work around this for now by Handler.posting the removeStateChangeCompletionListener call so it happens outside the loop. (This foreground stuff would probably be better as a service that implements ScopedServices.Activated, but this is how my code is currently structured)

Thanks for the library, it's really nice to use, a big improvement over Jetpack ViewModel!

[Zhuinden]

Oof, that's an oversight on my part. My mistake on using an enhanced for-loop while using remove, instead of iterating backwards.

The solution to this sort of issue is always to iterate backwards by index.

[Zhuinden]

Should be fixed by https://github.com/Zhuinden/simple-stack/commit/c7b937c7a5430ec3dc211d01e12158d95e430104 and released as 2.6.5

[Zhuinden]

Please report back if it solved your issue

[angusholder]

Yup that's solved it, thanks a lot for the fast fix!

[Zhuinden]

glad to hear :)