okio icon indicating copy to clipboard operation
okio copied to clipboard

NullPointerException when returning null from inside read/write

Open sky-ricardocarrapico opened this issue 3 years ago • 1 comments

We have this code:

fileSystem.read(path) {
    try {
        moshi.adapter(clazz).fromJson(this)
    } catch (e: JsonDataException) {
        // If the file is malformed, treat it the same as if we didn't have one.
        null
    }
}

When we enter the catch block, we'll get a NullPointerException because read/write uses the use function inside okio and on that function there's this return result!!.

I know I can switch to this:

try {
    fileSystem.read(path) {
        moshi.adapter(clazz).fromJson(this)
    }
} catch (e: JsonDataException) {
    // If the file is malformed, treat it the same as if we didn't have one.
    null
}

But I'm wondering if not being able to return null from inside use is intended or not.

Thanks!

sky-ricardocarrapico avatar Feb 01 '22 14:02 sky-ricardocarrapico

That's a mistake. Can fix.

swankjesse avatar Feb 17 '22 12:02 swankjesse

I am new to here, and I am thinking that the use function in Okio.kt could be replace by use in kotlin.io, its implement is

// kotlin.io/Closeable.kt
@InlineOnly
@RequireKotlin("1.2", versionKind = RequireKotlinVersionKind.COMPILER_VERSION, message = "Requires newer compiler version to be inlined correctly.")
public inline fun <T : Closeable?, R> T.use(block: (T) -> R): R {
    contract {
        callsInPlace(block, InvocationKind.EXACTLY_ONCE)
    }
    var exception: Throwable? = null
    try {
        return block(this)
    } catch (e: Throwable) {
        exception = e
        throw e
    } finally {
        when {
            apiVersionIsAtLeast(1, 1, 0) -> this.closeFinally(exception)
            this == null -> {}
            exception == null -> close()
            else ->
                try {
                    close()
                } catch (closeException: Throwable) {
                    // cause.addSuppressed(closeException) // ignored here
                }
        }
    }
}

I tried to replace the current code base with above, it passed both OkioKotlinTest.kt and OkioTest.java, but I am not sure is there anything I missed.

Kenny50 avatar Sep 26 '22 09:09 Kenny50

No further action on this.

swankjesse avatar Jan 14 '23 20:01 swankjesse