lateinit property not assigned value soon enough?

Question

My code works 9,999 out of 10,000 times. But, for about once in every 10,000 times, the application crashes to a UninitializedPropertyAccessException This is problematic as there are about 20,000~30,000 Android devices with my code in production.

It appears that, sometimes, the actual assignment of the lateinit variable is not happening soon enough (and hence causing the exception above).

Has anybody else had a similar problem? What was your solution?

TcpService.kt

class TcpService: Service() {

    private lateinit var mTcpClient: TcpClient

    override fun onCreate() {
        registerReceiver(object: BroadcastReceiver() {
            override fun onReceive(context: Context, intent: Intent) {
                when(intent.action) {
                    // This will cause an Uninitialized Property Exception 1 in 10,000 times
                    ACTION_SEND_MESSAGE -> mTcpClient.sendMessageAsync(intent.getStringExtra(EXTRA_NEW_MESSAGE)
                }
            }
        }, IntentFilter(ACTION_SEND_MESSAGE)
        })
    }

    override fun onStart() {
        mTcpClient = mTcpClient()
    }

    // ...

}

TcpClient.kt

class TcpClient() {

    init {
        // ...
        sendBroadcast(Intent(ACTION_SEND_MESSAGE).putExtra(EXTRA_NEW_MESSAGE, "Message Contents")
    }

    // ...

}

The concurrent broadcast is taking place before the `TcpClient` object initialization. — Alireza Ghasemi, Sep 20 '20 at 18:10
Why don't you move the instantiation before creating the receiver in `onCreate()`? — Tenfour04, Sep 20 '20 at 18:25
@Tenfour04 That would seem like the obvious solution, but this post is an extremely boiled-down version of the real code, where I can't instantiate TcpClient before the receiver — El Sushiboi, Sep 20 '20 at 18:51
@Alirezaa Thanks for pointing that out -- that probably is what I'm doing wrong. I've refactored my code to NOT have the broadcast in the init block. Only time will tell if this is the right solution, but I'm optimistic :) — El Sushiboi, Sep 20 '20 at 18:53
Would it be bad to have a ``start()`` method on the ``TcpClient``, and put that ``sendBroadcast`` bit in there? What you're sort of doing here is calling a method on the instance before the constructor (including the ``init`` block) has finished running - usually there's an async delay through the broadcast system, but I guess sometimes it just runs directly and immediately? By making it a separate step that you call after `mTcpClient = mTcpClient()` the field will definitely be assigned — cactustictacs, Sep 20 '20 at 23:32

score 0 · Answer 1 · answered Sep 20 '20 at 18:59

ahh it happened to me once and i wasted whole day then i solved it(still dont know how) but why dont you make mTcpClient null and then null check:

 `when(intent.action) {
 // This will cause an Uninitialized Property Exception 1 in 10,000 times
                ACTION_SEND_MESSAGE ->{
if(mTcpClient != null){
mTcpClient.sendMessageAsync(intent.getStringExtra(EXTRA_NEW_MESSAGE)  
}else{
mTcpClient=mTcpClient()
mTcpClient.sendMessageAsync(intent.getStringExtra(EXTRA_NEW_MESSAGE)
}
  }
    }`

something like that im not a professional sorry

score 0 · Answer 2 · answered Sep 20 '20 at 20:08

0

Instead of making the property null and then null check which can be dangerous in some scenarios, you can use isInitialized method on your lateInit properties :

    when (intent.action) {
        if (::mTcpClient.isInitialized) { ACTION_SEND_MESSAGE ->
          mTcpClient.sendMessageAsync(intent.getStringExtra(EXTRA_NEW_MESSAGE)
        }
    }

answered Sep 20 '20 at 20:08

Amirhosein

1,048
7
19

I'd say using a `lateinit` variable and asking if it is initialized is a bad practice or a code smell. It's preferable to use an optional type and a null check. – fernandospr Sep 20 '20 at 20:35
personally I think ``isInitialized`` is cleaner than making a non-null field nullable just so you can delay its initialisation, especially in something like Kotlin where you'll have to write all your code around that nullability (or pepper it with `!!` which is definitely a code smell) – cactustictacs Sep 20 '20 at 23:15
you could just set an ``initialised = true`` flag in this case though! – cactustictacs Sep 20 '20 at 23:21

lateinit property not assigned value soon enough?

TcpService.kt

TcpClient.kt

2 Answers2