IllegalThreadStateException OneSignal crash workaround

Question

Our app daily receives around 1k crashes based on bug mentioned on OneSignal's github issues.

---

Bug explanation:

Unfortunately, I can't reproduce this issue. All crashes come from Crashlytics reports. SDK version 3.12.4

Devices:

1) Samsung: Galaxy A5(2017), Galaxy S8, Galaxy A50, Galaxy S10+, Galaxy S10  
2) Xiaomi: Mi A2, Mi A2 lite, Mi A1, Mi A3, Redmi Note 5 Pro 
3) Oneplus: ONEPLUS A6010, OnePlus5T, GM191011, GM19008, OnePlus58

Stacktrace:

Caused by java.lang.IllegalThreadStateException
       at java.lang.Thread.start(Thread.java:724)
       at com.onesignal.OneSignalPrefs$WritePrefHandlerThread.startDelayedWrite(OneSignalPrefs.java:117)
       at com.onesignal.OneSignalPrefs.startDelayedWrite(OneSignalPrefs.java:183)
       at com.onesignal.OneSignal.setAppContext(OneSignal.java:601)
       at com.onesignal.OneSignalSyncServiceUtils.doBackgroundSync(OneSignalSyncServiceUtils.java:175)
       at com.onesignal.SyncJobService.onStartJob(SyncJobService.java:40)
       at android.app.job.JobService$1.onStartJob(JobService.java:62)
       at android.app.job.JobServiceEngine$JobHandler.handleMessage(JobServiceEngine.java:108)
       at android.os.Handler.dispatchMessage(Handler.java:106)
       at android.os.Looper.loop(Looper.java:280)
       at android.app.ActivityThread.main(ActivityThread.java:6748)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

---

Question:

The main problem is that they tagged it as medium priority and bug exists around 3 months. Our vitals are going for a toss, just because of this issue. It's costing us a lot.

Does there exist any workaround that can temporary solve the problem?

---

P.S:

I'm ready to provide more related info, if required. Thanks in advance!

Answer 1

It is possible(if not likely) that you have an old version of OneSignal where synchronized void startDelayedWrite() is not synchronized. In this case I would update, and see if that fixes things. If not see the following:

The exception in question occurs here:

synchronized void startDelayedWrite() {
            if (mHandler == null) {
                start();
                mHandler = new Handler(getLooper());
            }
//rest of function irrelevant.

The Javadoc for Thread.start,states the following:

It is never legal to start a thread more than once. In particular, a thread may not be restarted once it has completed execution.

From this and the implementation of Thread.start, we can infer that the thread in question is being started twice.

For this to be started twice in startDelayedWrite, new Handler(getLooper()) needs to throw an exception, otherwise we won't enter this if statement again. I don't see any way for getLooper() to throw an exception, but new Handler, certainly can if getLooper() is null.

The implementation of getLooper() is as follows:

public Looper getLooper() {
        if (!isAlive()) {
            return null;
        }
//rest omitted b/c irrelevant

The only way the thread in question could have exited is if Thread.run exited early.

The thread's run implementation looks like this:

@Override
    public void run() {
        mTid = Process.myTid();
        Looper.prepare();
        synchronized (this) {
            mLooper = Looper.myLooper();
            notifyAll();
        }
        Process.setThreadPriority(mPriority);
        onLooperPrepared();
        Looper.loop();
        mTid = -1;
    }

Looper.loop, is intended to be a semi-infinite loop which reads from a message queue.

Note the following in Looper.loop:

for (;;) {
            Message msg = queue.next(); // might block
            if (msg == null) {
                // No message indicates that the message queue is quitting.
                return;
            }

So if msg == null, then our thread exits quickly, possibly causing an exception in mHandler = new Handler(getLooper());, resulting in Thread.start being called twice.

There are other plausible explanations. For example it is possible that Looper.loop crashes at some earlier point.

To mitigate this I would add a try{} finally{} block around mHandler = new Handler(getLooper());, to handle the case where the Looper has already exited. Alternatively I may be wrong and this race condition may be caused by something completely different.