we received a crash on Firebase for a kotlin method:

Fatal Exception: java.lang.NullPointerException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkNotNullParameter, parameter code
       at [redacted].DeliveryMethod.<init>(:2)
       at [redacted].DeliveryMethodsUpdater$addSingleDMInAd$clientCall$1.invokeSuspend(DeliveryMethodsUpdater.kt:121)
       at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
       at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
       at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)

the model is this one:

class DeliveryMethod() {
    lateinit var code: String
    lateinit var name: String
    lateinit var description: String

var isAddressRequired: Boolean? = null
var image: JSONObject? = null
var isDefault: Boolean = false

constructor(code: String) : this() {
    this.code = code
}

constructor(code: String, name: String, description: String, image: JSONObject? = null) : this() {
    this.code = code
    this.name = name
    this.description = description
    this.image = image
}
}

and the method:

private suspend fun addSingleDMInAd(
        adId: Long,
        deliveryMethodCode: String
    ): JoinAdDeliveryMethod? {
        var addedDeliveryMethod: JoinAdDeliveryMethod? = null
        val clientCall = GlobalScope.async(Dispatchers.IO) {
            val cd = CountDownLatch(1)
            Client.getInstance().addDeliveryMethodInAd(
                adId,
                DeliveryMethod(deliveryMethodCode),
                object : NetworkCallback<JoinAdDeliveryMethod> {
                    override fun onSuccess(result: JoinAdDeliveryMethod) {
                        addedDeliveryMethod = result
                        cd.countDown()
                    }

                    override fun onFailure(err: NetworkError?) {
                        addedDeliveryMethod = null
                        cd.countDown()
                    }
                }
            )
            cd.await()
        }
        clientCall.await()
        return addedDeliveryMethod
    }

now, I understand that that the constructor for DeliveryMethod is being called with a null value for code, but I don't understand why the exception only come up at this point. As you can see, the method param is also marked as non-null, and so are previous methods. Shouldn't the exception be thrown way before getting to the constructor call for DeliveryMethod?

CodePudding user response：

Shouldn't the exception be thrown way before getting to the constructor call for DeliveryMethod?

Within Kotlin, it's not possible for a non-null parameter to be given a null value at runtime (because the code wouldn't have compiled in the first place). However, this can happen if the value is passed from Java.

This is why the Kotlin compiler generates null-checks (with the intrinsic checkNotNullParameter you're seeing fail here) only in non-suspend public/protected/internal methods, to prevent misuse from Java. There is no point to do that in private or suspend methods since they can only be called from Kotlin (usually), and it would add some overhead that might not be acceptable in performance-sensitive code.

This is why calling addSingleDMInAd doesn't itself fail with this error. That said, it would be interesting to see how you're getting the null here, because usually the checks at the public API surface are enough. Is some reflection or unsafe cast involved here?

Also, the way your model is setup is quite strange. It seems the lateinit is lying because depending on which constructor is used, the properties may actually not be set at all. It would be safer to mark them as nullable to account for when users of that class don't set the value of these properties. Doing this, you won't even need all secondary constructors, and you can just use default values:

class DeliveryMethod() {
    var code: String? = null,
    var name: String? = null,
    var description: String? = null,
    var image: JSONObject? = null,
) {
    var isAddressRequired: Boolean? = null
    var isDefault: Boolean = false
}

Other things of note about addSingleDMInAd:

• don't use GlobalScope in this case. If you need to run short-lived coroutines, provide them with a smaller scope that is cancelled when the work is not needed anymore - it ensures no coroutines are leaked. You can read more about the potential pitfalls of GlobalScope and possible replacements in its own doc. That said, you probably shouldn't be starting a coroutine at all anyway here, see next point.

• don't use async {} if you use await() right after - it's pointless to start something asynchronous if you wait for it right there. If you want to switch the context to IO, use withContext(Dispatchers.IO) { ... } instead. That said, you shouldn't even need to use the IO dispatcher here, see next point.

• don't use CountDownLatch for this purpose. The proper way to encapsulate an async API as a suspend function for coroutines is to use suspendCancellableCoroutine (check its doc, it provides an example on how to use it). Once you use this, there is no need anymore for Dispatchers.IO because it will not be blocking the current thread anymore.