2

I have a method, which loading image from Firebase Storage. It's called in background thread, and I need to block it, until image is loaded (to avoid callback hell). Here is the code (in Kotlin)

override fun fromNet(): Bitmap? {
    Log.wtf(TAG, "$name loading from firebase")
    var result: Bitmap? = null
    val lock = CountDownLatch(1)
    try {
        FirebaseStorage.getInstance().getReferenceFromUrl(FIRE_STORAGE).child(ctx.getKGL().famkey)
            .child(name).getBytes(524288L)
            .addOnFailureListener {
                Log.wtf(TAG, "$name load failure")
                lock.countDown()
            }
            .addOnSuccessListener { bytes ->
                Log.wtf(TAG, "$name loaded")
                val b = BitmapFactory.decodeByteArray(bytes, 0, bytes.size).scale(ctx.dip(64))
                result = b
                lock.countDown()
                ctx.saveToCache(name, b)
            }
            .addOnCompleteListener {
                Log.wtf(TAG, "on complete")
                lock.countDown()
            }
    } catch (ignored: NullPointerException) { lock.countDown() }
    lock.await()
    return result
}

But thread stays blocked forever

Logcat:

A/MemberPhoto: xvd6z67gZfMCLG4c9mkGXKe9ML53 load failure
A/MemberPhoto: on complete

UPD: May the cause be, that Firebase code is Java, and my code is in Kotlin?

Dmytro Rostopira
  • 10,588
  • 4
  • 64
  • 86
  • Well, `lock.await()` will never unblock at least if `NullPointerException` is thrown in the `try` block before `lock.countDown()` is called. Isn't it the case? – hotkey Oct 28 '16 at 17:01
  • @hotkey I thought, if exception is thrown - await() won't be called. But let me check – Dmytro Rostopira Oct 28 '16 at 17:05
  • My bad, I didn't spot the `return null` in the `catch` block. Sorry. – hotkey Oct 28 '16 at 17:07
  • I replaced `return null` with `lock.countdown` - still an issue – Dmytro Rostopira Oct 28 '16 at 17:15
  • do you see the log messages in your log, indicating that the listeners have been called? if so which message do you get? – Nicolas Filotto Oct 30 '16 at 18:13
  • you should modify your code to call `lock.countDown()` in a `finally` block, indeed here if `BitmapFactory.decodeByteArray(bytes, 0, bytes.size).scale(ctx.dip(64))` fails, `lock.countDown()` is never called – Nicolas Filotto Oct 30 '16 at 18:25
  • 2
    @NicolasFilotto if he calls `lock.countDown()` in the finally block then it will never wait. the finally is on his main thread that is supposed to wait. – Jayson Minard Oct 31 '16 at 00:28
  • @DimaRostopira can you show the logged messages as requested, they are evidence. – Jayson Minard Oct 31 '16 at 00:30
  • @JaysonMinard I believe that you missunderstand my proposal, what I propose is to put code of listeners in try/finally blocks and call lock.countDown() in the finally blocks to ensure that it will be called whatever happens – Nicolas Filotto Oct 31 '16 at 06:43
  • @NicolasFilotto I see messages in logcat, and can catch this lines with debugger. All three listeners are called, onComplete always, onFailure and onSuccess depends on result. Also I put `lock.countDown()` in catch block, updated code – Dmytro Rostopira Oct 31 '16 at 08:46
  • @NicolasFilotto ah, makes sense if the failures are within the callbacks. Good plan. – Jayson Minard Oct 31 '16 at 12:30
  • @DimaRostopira the last proposal of Nicolas was to make sure within each callback you have a `try...finally { lock.countDown() }` so that any errors during the callback don't prevent the countDown call (which would cause this to lockup). it is likely your only hole in the code if something like `decodeByteArray` fails with exception. – Jayson Minard Oct 31 '16 at 12:32
  • @JaysonMinard then there will be no wait, firebase spawns new thread and finally will be called before file loaded. But I will try surround `decode...` with try catch, but this doesn't makes sense, since onCompleted called always – Dmytro Rostopira Oct 31 '16 at 12:39

1 Answers1

4

If you want to be sure that lock.await() won't make your current thread wait forever, you need to ensure that lock.countDown() is called whatever happens so here you should surround with a try/finally block the code of your listeners in order to call lock.countDown() within a finally block.

Indeed otherwise with your current code if for example BitmapFactory.decodeByteArray(bytes, 0, bytes.size).scale(ctx.dip(64)) fails, lock.countDown() will never be called which will make the thread calling lock.await() wait forever.

For example in case of a success the code of your listener should rather be:

.addOnSuccessListener { bytes ->
    try {
        Log.wtf(TAG, "$name loaded")
        val b = BitmapFactory.decodeByteArray(bytes, 0, bytes.size).scale(ctx.dip(64))
        result = b
    } finally {
        lock.countDown()
    }        
    ctx.saveToCache(name, b)
}
Nicolas Filotto
  • 43,537
  • 11
  • 94
  • 122