Fix for a crash in logout (closes #263)

[davidvavra]

We see crashes in logout in our production app. Here is the stacktrace:

Fatal Exception: java.lang.IllegalStateException
FragmentManager has been destroyed
androidx.fragment.app.FragmentManager.enqueueAction (FragmentManager.java:1542)

androidx.fragment.app.BackStackRecord.commit (BackStackRecord.java:306)
com.okta.oidc.OktaResultFragment.addLogoutFragment (OktaResultFragment.java:64)
com.okta.oidc.clients.web.SyncWebAuthClientImpl.lambda$startSignOut$4 (SyncWebAuthClientImpl.java:321)
com.okta.oidc.clients.web.SyncWebAuthClientImpl.$r8$lambda$9xGAYUir3zQW6WeNxzc89L77keM (SyncWebAuthClientImpl.java)
com.okta.oidc.clients.web.SyncWebAuthClientImpl$$InternalSyntheticLambda$0$d787f1d4a5cbac304e1d1e8dca806be8b9084ceb4a03a44ad24a9ec426c49b62$0.run (SyncWebAuthClientImpl.java:6)

The app crashes, because FragmentManager is destroyed, but the SDK is trying to commit a Fragment transaction.

I'm not sure about the root cause, but the fix is a simple check if FragmentManager is destroyed. It should not crash in this case and normal functionality should not be affected.

Closes this issue: https://github.com/okta/okta-oidc-android/issues/263

[JayNewstrom]

Thanks for the PR. Could you sign our CLA? https://developer.okta.com/cla/

We will also want to add a test for this. Could you do that?

[davidvavra]

I have already signed SLA with my first pull request. Is it required to sign it for every PR?

I added the test for this.

[JayNewstrom]

Sorry, we don't have an easy way to check for CLAs, one signature is enough. Thanks.

[JayNewstrom]

I'm also not in love with this fix. We don't have any way to notify the caller that their logout failed, and didn't do what they expected.

I think this could be detected in application logic by checking if the activity is finished before calling into the Okta SDK.

[davidvavra]

This is how we call Okta SDK for logout:

1) client.registerCallback(...) 2) client.signOutOfOkta(activity) 3) When the callback is triggered (same handling for all cases): 4) client.sessionClient?.clear() 5) activity.finish()

So I don't see a way how we could check if activity is finished before calling Okta SDK. I think this check needs to be inside Okta SDK. Do you have some suggestions how to do it better? I don't have a big knowledge of internals of the SDK and didn't want to change it too much.

[JayNewstrom]

For step 2, could you wrap that in an if (!activity.isFinished()) { call?

[davidvavra]

You might be right, we never finish the activity before we get callback from the SDK. But the user could do that. I will add that if and let you know in the issue if that stopped the crashes. I will close this PR for now.

[JayNewstrom]

Thank you! And thanks again for putting the time in to post a PR. Really appreciate it.

[ashish-d-hh]

Did this fixed the crash for Fragment manager has been destroyed for Okta ?

[JayNewstrom]

Hi @ashish-d-hh please see #263 as we're still working through it in that issue.

[davidvavra]

Reopening based on discussion in the issue. @JayNewstrom Could you help with the unit test? I don't have that much knowledge about your test setup.

[JayNewstrom]

@davidvavra Sure. If you want to remove the test portion of the PR, I can merge that, and follow up with a test in another PR.

[davidvavra]

Thanks, reverted.