metaplex-foundation / metaplex-android

Metaplex Mobile Android SDK
18 stars 12 forks source link

Sample App Crash - attempting to use destroyed context #49

Open Funkatronics opened 2 years ago

Funkatronics commented 2 years ago

Summary

During testing I have now seen the sample app crash several times when trying to open the NFT details screen. The crash is caused do to improper use of a Context instance that is no longer valid.

java.lang.IllegalArgumentException: You cannot start a load for a destroyed activity
        at com.bumptech.glide.manager.RequestManagerRetriever.assertNotDestroyed(RequestManagerRetriever.java:353)
        at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:153)
        at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:133)
        at com.bumptech.glide.Glide.with(Glide.java:818)
        at com.metaplex.sample.NFTRecycleViewAdapter$onBindViewHolder$2$1.invoke$lambda-0(FirstFragment.kt:124)
        at com.metaplex.sample.NFTRecycleViewAdapter$onBindViewHolder$2$1.$r8$lambda$yvSIRD4fwJzOUi96RoJ85KaCCgg(Unknown Source:0)
        at com.metaplex.sample.NFTRecycleViewAdapter$onBindViewHolder$2$1$$ExternalSyntheticLambda0.run(Unknown Source:6)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

Cause

NFT metadata data loading is not lifecycle aware, leading to an improper use of an already destroyed context if the callback is triggered after the activity has been stopped.

metadata(metaplex) { result ->
            result.onSuccess {
                ...
                    Handler(Looper.getMainLooper()).post(Runnable {
                        Glide
                            .with(context) << context could be invalid here 
                         ... 
                    })
            }
        }

Impact

Its a sample app, it is not meant to be perfect. Likely will leave this bug as-is.

ajamaica commented 2 years ago

Give this to the fellow. @thedevyansh

thedevyansh commented 2 years ago

Summary

During testing I have now seen the sample app crash several times when trying to open the NFT details screen. The crash is caused do to improper use of a Context instance that is no longer valid.

java.lang.IllegalArgumentException: You cannot start a load for a destroyed activity
        at com.bumptech.glide.manager.RequestManagerRetriever.assertNotDestroyed(RequestManagerRetriever.java:353)
        at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:153)
        at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:133)
        at com.bumptech.glide.Glide.with(Glide.java:818)
        at com.metaplex.sample.NFTRecycleViewAdapter$onBindViewHolder$2$1.invoke$lambda-0(FirstFragment.kt:124)
        at com.metaplex.sample.NFTRecycleViewAdapter$onBindViewHolder$2$1.$r8$lambda$yvSIRD4fwJzOUi96RoJ85KaCCgg(Unknown Source:0)
        at com.metaplex.sample.NFTRecycleViewAdapter$onBindViewHolder$2$1$$ExternalSyntheticLambda0.run(Unknown Source:6)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

Cause

NFT metadata data loading is not lifecycle aware, leading to an improper use of an already destroyed context if the callback is triggered after the activity has been stopped.

metadata(metaplex) { result ->
            result.onSuccess {
                ...
                    Handler(Looper.getMainLooper()).post(Runnable {
                        Glide
                            .with(context) << context could be invalid here 
                         ... 
                    })
            }
        }

Impact

Its a sample app, it is not meant to be perfect. Likely will leave this bug as-is.

While trying to use the findByMint(mint, callback) method, I found that it is returning NPE for many mint accounts. This is leading to the NFT details screen crashing.

java.lang.NullPointerException
        at com.solana.api.GetAccountInfoKt$getAccountInfo$1.invoke(getAccountInfo.kt:44)
        at com.solana.api.GetAccountInfoKt$getAccountInfo$1.invoke(getAccountInfo.kt:41)
        at com.solana.networking.NetworkingRouter$call$1.onResponse(NetworkingRouter.kt:105)
        at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:764)
Funkatronics commented 2 years ago

While trying to use the findByMint(mint, callback) method, I found that it is returning NPE for many mint accounts. This is leading to the NFT details screen crashing.

java.lang.NullPointerException
        at com.solana.api.GetAccountInfoKt$getAccountInfo$1.invoke(getAccountInfo.kt:44)
        at com.solana.api.GetAccountInfoKt$getAccountInfo$1.invoke(getAccountInfo.kt:41)
        at com.solana.networking.NetworkingRouter$call$1.onResponse(NetworkingRouter.kt:105)
        at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:764)

@thedevyansh This is a separate issue. We can log a separate ticket for this if needed.

cdhiraj40 commented 1 year ago

@ajamaica @Funkatronics I see a problem just by seeing the code which is handling on click an item, then creating the activity from the adapter using that context(item view's context) . https://github.com/metaplex-foundation/metaplex-android/blob/818a5df14facc1fd9fc35f9bc36ec43f5aaffeee/sample/src/main/java/com/metaplex/sample/FirstFragment.kt#L127

It should not be handled this way. Correct way to handle this would be using callbacks, and creating new activity from FirstFragment.kt itself. I will give it a try and see, most probably this should solve the error!