NullPointerException when I connect same address twice (double click the item)

[liqiwei1992]

Hello,

When I use andiodine, I got a null pointer exception when andiodine attempts to connect the same address twice if I click the item quickly.

I read the code snippets about Connect in IodineVpnService.java. I found that

private final BroadcastReceiver broadcastReceiver = new BroadcastReceiver() {
    @Override
    public void onReceive(Context context, Intent intent) {
    ...
    } else if (ACTION_CONTROL_CONNECT.equals(intent.getAction())) {
        if (mThread != null) {
            ...
            return;
        }
        ...
        //if OS schedules other threads at this point, it may lead some errors
        mThread = new Thread(IodineVpnService.this, IodineVpnService.class.getName());
        mThread.start();
        }
    }
};

I think those code may lead to synchronization errors if it calls method onReceive twice. Therefore, andiodine should take extra synchronization (such as Synchronized {...}) to fix it.

And the stack trace is:

E/AndroidRuntime( 7557): java.lang.NullPointerException: Attempt to invoke virtual method 'void android.app.Activity.sendBroadcast(android.content.Intent)' on a null object reference

E/AndroidRuntime( 7557):    at org.xapek.andiodine.FragmentList.vpnServiceConnect2(FragmentList.java:203)

E/AndroidRuntime( 7557):    at org.xapek.andiodine.FragmentList.access$600(FragmentList.java:30)

E/AndroidRuntime( 7557):    at org.xapek.andiodine.FragmentList$3.onClick(FragmentList.java:177)

E/AndroidRuntime( 7557):    at com.android.internal.app.AlertController$ButtonHandler.handleMessage(AlertController.java:162)

E/AndroidRuntime( 7557):    at android.os.Handler.dispatchMessage(Handler.java:102)

E/AndroidRuntime( 7557):    at android.os.Looper.loop(Looper.java:135)

E/AndroidRuntime( 7557):    at android.app.ActivityThread.main(ActivityThread.java:5254)

E/AndroidRuntime( 7557):    at java.lang.reflect.Method.invoke(Native Method)

E/AndroidRuntime( 7557):    at java.lang.reflect.Method.invoke(Method.java:372)

E/AndroidRuntime( 7557):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)

E/AndroidRuntime( 7557):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

[yvesf]

Hi,
thank you for the detailed bug report.

Regarding the error I believe the reason for it is different (did you try to prove the synchronized{}-theory?).
I believe the Fragment is loosing the activity (therefore getActivity() return null) once the IodineVpnService switches the application status to CONNECT (l. 210) because then the IodineMain switches the Fragments (l. 42).
I won't promise you a date when I fix it but if you would like to do it then a PR is very welcome.
Kind regads, yves

[liqiwei1992]

Hi,
I have read the IodineMain.java, FragmentList.java and IodineVpnService.java. I think you are totally right.

The event sequences to replay the crash is as follows:

1. Add a new Connection --> 2. click the item --> 3. click ok --> 4. click the item --> 5. click ok --> crash
   And the time interval between steps 3 and 4 should be very small.

The root cause may be like this. The first invocation of vpnServiceConnect2 sent an intent to broadcastReceiver which started a new thread (IodineVpnService). And this new thread sent new status to IodineMain which switched the Fragments. Therefore, the second invocation of vpnServiceConnect2 get a null pointer after invocation of getActivity.

Please point out mistakes in it.

Thanks!

ps. The onReceive of BroadcastReceiver is always called within the main thread of its process, unless you explicitly asked for it to be scheduled on a different thread using registerReceiver(BroadcastReceiver, IntentFilter, String, Handler). Therefore, there won't be synchronized problems.

[yvesf]

My phone is too fast, I don't get a chance to click the item a second time 😃
If you have a fix for this then feel free to open a PR