RedApparat / Fotoapparat

Making Camera for Android more friendly. 📸
Apache License 2.0
3.82k stars 407 forks source link

PendingResult.whenAvailable callback not called on main thread #318

Open bradroid opened 5 years ago

bradroid commented 5 years ago

I was using whenAvailable for a while and it worked fine. Today I updated Fotoapparat to latest version (2.6.1), along with other dependency updates in the same project (e.g. bumped Glide to 4.8 - could be relevant).

It seems like whenAvailable callback is not really called on the main thread. Glide has utility function which checks for main thread and throws if it returns false. Because whenAvailable's callback eventually reaches image loading code with Glide, my app crashes.

I copied Glide's main thread checking code to whenAvailable call site to create a short code block which demonstrates the issue:

val photoResult = fotoapparat.autoFocus().takePicture()
val file = ImageFileUtils.createImageFile(saveDirectoryPath)
        photoResult.saveToFile(file)
                .whenAvailable {
                    if (Looper.myLooper() != Looper.getMainLooper()) {
                        throw IllegalStateException("Such error. Wow." + Looper.myLooper())
                    }
                    val photoFilePath = file.absolutePath
                    presenter.onPhotoTaken(photoFilePath)
                }

The Looper.myLooper() returns null , so isMainThread utility functions fail because they are usually implemented like: return (Looper.myLooper() == Looper.getMainLooper()).

In the debug mode, this is what "current thread" looks like when I'm in the main thread (e.g Activity's onCreate). screen shot 2018-11-08 at 5 38 03 pm

And this is what "current thread" looks like when I set the breakpoint within whenAvailable's callback:

screen shot 2018-11-08 at 5 38 40 pm

Diolor commented 5 years ago

I think something's wrong with your code. Neither Looper.myLooper() nor Looper.getMainLooper() should return null. And if really both were null, the exception wouldn't have been thrown. Maybe you import another Looper class other than the android.os.Looper one? Or it's stubbed?

Sample project looks good here: image

bradroid commented 5 years ago

I can see that the issue happens on emulators but not on real device for my project, I will try running your sample on same emulators.

bradroid commented 5 years ago

Yeah on Android emulator I'm getting this in Logcat for your sample: io.fotoapparat.sample E/Fotoapparat Example: Couldn't capture photo.

When I set the breakpoint inside whenAvailable i can see that photo is null and also Looper.myLooper() returns null. Don't know why, but I can live with this being emulator only issue.

I used Pixel API 28

Diolor commented 5 years ago

Interestingly my screenshot above was from an emu "Pixel API 28 w/ Google play" in our sample.

Can you please check the sample project we have in your emulator? Is it reproducible there too?

bradroid commented 5 years ago

Hi, my previous comment was with your sample. I cloned your repo and could reproduce. Maybe something is messed up with my emulator don't know.

strooooke commented 5 years ago

We're seeing now (no idea how long, we just set up pre-launch reports with this release) the same problem in pre-launch reports in the Play Store (in our own app). In our case, it's not Glide doing and failing the check, but actual framework View thread checking at

android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
    at android.view.ViewRootImpl.checkThread(ViewRootImpl.java:7313)
    at android.view.ViewRootImpl.recomputeViewAttributes(ViewRootImpl.java:3462)

We cannot reproduce this, neither with actual devices nor with emulator.

Looks like it happened for any device where the random pre-launch exploration has reached this point in our app (7 of 10 devices; various models and API levels).

The only thought I've got is that maybe in this circumstance, UIThread is different from MainThread.

ashu-dadhich commented 5 years ago

We're seeing now (no idea how long, we just set up pre-launch reports with this release) the same problem in pre-launch reports in the Play Store (in our own app). In our case, it's not Glide doing and failing the check, but actual framework View thread checking at

android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
  at android.view.ViewRootImpl.checkThread(ViewRootImpl.java:7313)
  at android.view.ViewRootImpl.recomputeViewAttributes(ViewRootImpl.java:3462)

We cannot reproduce this, neither with actual devices nor with emulator.

Looks like it happened for any device where the random pre-launch exploration has reached this point in our app (7 of 10 devices; various models and API levels).

The only thought I've got is that maybe in this circumstance, UIThread is different from MainThread.

We are facing a similar issue for our play-store app.

bradroid commented 5 years ago

@Diolor I was able to reproduce the issue on real device so I did more research and found the source. I was able to inconsistently reproduce this on Samsung galaxy s7 edge. I was taking black photos with my phone on the desk face down(camera turned to desk) without the flash turned on. Don't know if this is even relevant. Crash would occur randomly every 5-15 photos taken.

Whenever I reproduced the issue i could see ""Couldn't deliver pending result: Operation failed internally." message in the logcat. The issue is in whenAvailable method in the last catch block. You're calling callback(null) without switching to main thread first.