Opened 7 weeks ago

Closed 6 weeks ago

#2629 closed defect (fixed)

TunnelEntry.createNewTunnel java.lang.NullPointerException

Reported by: Meeh Owned by: Meeh
Priority: minor Milestone: 0.9.43
Component: apps/android Version: 0.9.42
Keywords: Cc:
Parent Tickets: Sensitive: no

Description

java.lang.RuntimeException: 
  at android.app.ActivityThread.performResumeActivity (ActivityThread.java:3320)
  at android.app.ActivityThread.handleResumeActivity (ActivityThread.java:3351)
  at android.app.ActivityThread.handleLaunchActivity (ActivityThread.java:2665)
  at android.app.ActivityThread.-wrap11 (ActivityThread.java)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1494)
  at android.os.Handler.dispatchMessage (Handler.java:111)
  at android.os.Looper.loop (Looper.java:207)
  at android.app.ActivityThread.main (ActivityThread.java:5741)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:789)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:679)
Caused by: java.lang.RuntimeException: 
  at android.app.ActivityThread.deliverResults (ActivityThread.java:3935)
  at android.app.ActivityThread.performResumeActivity (ActivityThread.java:3302)
Caused by: java.lang.NullPointerException: 
  at net.i2p.android.i2ptunnel.TunnelEntry.createNewTunnel (TunnelEntry.java:40)
  at net.i2p.android.i2ptunnel.TunnelsContainer.onActivityResult (TunnelsContainer.java:220)
  at android.support.v4.app.FragmentActivity.onActivityResult (FragmentActivity.java:160)
  at android.app.Activity.dispatchActivityResult (Activity.java:6503)
  at android.app.ActivityThread.deliverResults (ActivityThread.java:3931)

Subtickets

Change History (6)

comment:1 Changed 6 weeks ago by zzz

TCG.getControllers() returns non-null so tcg must be null.
Changes were made in TCG in .41, see TCG.getInstance() javadocs. Looks like these changes triggered the issue?

comment:2 Changed 6 weeks ago by zzz

Summary of TCG API for Android as of 41:

  • getInstance() does not initialize and may return null.
  • getInstance(context) will initialize if necessary and always returns non-null

all getInstance() calls must either do null check afterwards, or be converted to getInstance(context)

comment:3 Changed 6 weeks ago by Meeh

Status: newtesting

Okay, I did what I think is best without altering strings, if you think I should add a customised message for this I can of course update it.

--- app/src/main/java/net/i2p/android/i2ptunnel/TunnelEntry.java	7e91eb3ac47833452015e22c2773be54bf2bea0b
+++ app/src/main/java/net/i2p/android/i2ptunnel/TunnelEntry.java	830dc298c4ca6a6fb6cdbca835c32f3c9937b0af
@@ -37,14 +37,17 @@ public class TunnelEntry {
             Context ctx,
             TunnelControllerGroup tcg,
             TunnelConfig cfg) {
-        int tunnelId = tcg.getControllers().size();
         TunnelEntry ret = null;
         List<String> msgs = new ArrayList<>();
         SaveTunnelTask task = new SaveTunnelTask(tcg, -1, cfg);
         try {
+            int tunnelId = tcg.getControllers().size();
             msgs.addAll(task.execute().get());
             TunnelController cur = TunnelUtil.getController(tcg, tunnelId);
             ret = new TunnelEntry(ctx, cur, tunnelId);
+        } catch (NullPointerException e) {
+            Util.e("TunnelControllerGroup don't exists yet", e);
+            msgs.add(ctx.getString(R.string.i2ptunnel_msg_config_save_failed));
         } catch (InterruptedException e) {
             Util.e("Interrupted while saving tunnel config", e);
             msgs.add(ctx.getString(R.string.i2ptunnel_msg_config_save_failed));

comment:4 Changed 6 weeks ago by zzz

Milestone: undecided0.9.43
Status: testingneeds_work

It's almost never correct to fix an NPE by catching the NPE. Proposed fix below does the null check in TunnelsContainer?. Additionally, it fixes all the getInstance() calls to pass the context to take advantage of the .41 API as suggested in comment 2.

============================================================
--- app/src/main/java/net/i2p/android/i2ptunnel/TunnelDetailFragment.java	2e6425a1af65f20c135c82b237bfed89a6607478
+++ app/src/main/java/net/i2p/android/i2ptunnel/TunnelDetailFragment.java	57791d67e9c062654ad2980b0d7094b797fb436d
@@ -79,7 +79,7 @@ public class TunnelDetailFragment extend
         String error;
         List<TunnelController> controllers;
         try {
-            mGroup = TunnelControllerGroup.getInstance();
+            mGroup = TunnelControllerGroup.getInstance(I2PAppContext.getGlobalContext());
             error = mGroup == null ? getResources().getString(R.string.i2ptunnel_not_initialized) : null;
             controllers = mGroup == null ? null : mGroup.getControllers();
         } catch (IllegalArgumentException iae) {
============================================================
--- app/src/main/java/net/i2p/android/i2ptunnel/TunnelEntry.java	830dc298c4ca6a6fb6cdbca835c32f3c9937b0af
+++ app/src/main/java/net/i2p/android/i2ptunnel/TunnelEntry.java	aa5c96e2439f0c9d1a6a8421f93e935912a912f8
@@ -31,23 +31,21 @@ public class TunnelEntry {
     private final int mId;
 
     /**
+     * @param tcg non-null
      * @return the new TunnelEntry, or null if there was an error.
      */
     public static TunnelEntry createNewTunnel(
             Context ctx,
             TunnelControllerGroup tcg,
             TunnelConfig cfg) {
+        int tunnelId = tcg.getControllers().size();
         TunnelEntry ret = null;
         List<String> msgs = new ArrayList<>();
         SaveTunnelTask task = new SaveTunnelTask(tcg, -1, cfg);
         try {
-            int tunnelId = tcg.getControllers().size();
             msgs.addAll(task.execute().get());
             TunnelController cur = TunnelUtil.getController(tcg, tunnelId);
             ret = new TunnelEntry(ctx, cur, tunnelId);
-        } catch (NullPointerException e) {
-            Util.e("TunnelControllerGroup don't exists yet", e);
-            msgs.add(ctx.getString(R.string.i2ptunnel_msg_config_save_failed));
         } catch (InterruptedException e) {
             Util.e("Interrupted while saving tunnel config", e);
             msgs.add(ctx.getString(R.string.i2ptunnel_msg_config_save_failed));
============================================================
--- app/src/main/java/net/i2p/android/i2ptunnel/TunnelListFragment.java	3f70d124ba4bb0562a769715e9ddc60bde036d03
+++ app/src/main/java/net/i2p/android/i2ptunnel/TunnelListFragment.java	98d291b8ff4f1ad51921366409ae6eee47018210
@@ -19,6 +19,7 @@ import com.pnikosis.materialishprogress.
 
 import com.pnikosis.materialishprogress.ProgressWheel;
 
+import net.i2p.I2PAppContext;
 import net.i2p.android.router.R;
 import net.i2p.android.router.service.RouterService;
 import net.i2p.android.router.service.State;
@@ -170,7 +171,7 @@ public class TunnelListFragment extends 
     private void initTunnels() {
         if (mGroup == null) {
             try {
-                mGroup = TunnelControllerGroup.getInstance();
+                mGroup = TunnelControllerGroup.getInstance(I2PAppContext.getGlobalContext());
             } catch (IllegalArgumentException iae) {
                 Util.e("Could not load tunnels", iae);
                 mGroup = null;
============================================================
--- app/src/main/java/net/i2p/android/i2ptunnel/TunnelsContainer.java	44f515edcd628e78acdab430f7bb6c425a1b4a46
+++ app/src/main/java/net/i2p/android/i2ptunnel/TunnelsContainer.java	012b82fc2f1618fae8a9e8106a1383e8a1acb5a0
@@ -21,6 +21,7 @@ import com.viewpagerindicator.TitlePageI
 
 import com.viewpagerindicator.TitlePageIndicator;
 
+import net.i2p.I2PAppContext;
 import net.i2p.android.i2ptunnel.preferences.EditTunnelContainerFragment;
 import net.i2p.android.i2ptunnel.util.TunnelUtil;
 import net.i2p.android.router.R;
@@ -67,7 +68,7 @@ public class TunnelsContainer extends Fr
 
     private boolean showActions() {
         RouterContext rCtx = Util.getRouterContext();
-        TunnelControllerGroup tcg = TunnelControllerGroup.getInstance();
+        TunnelControllerGroup tcg = TunnelControllerGroup.getInstance(I2PAppContext.getGlobalContext());
         return rCtx != null && tcg != null &&
                 (tcg.getState() == ClientAppState.STARTING ||
                         tcg.getState() == ClientAppState.RUNNING);
@@ -177,7 +178,7 @@ public class TunnelsContainer extends Fr
 
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
-        TunnelControllerGroup tcg = TunnelControllerGroup.getInstance();
+        TunnelControllerGroup tcg = TunnelControllerGroup.getInstance(I2PAppContext.getGlobalContext());
         if (tcg == null)
             return false;
 
@@ -215,7 +216,9 @@ public class TunnelsContainer extends Fr
                 if (tunnelData == null)
                     return;
                 // TODO fetch earlier
-                TunnelControllerGroup tcg = TunnelControllerGroup.getInstance();
+                TunnelControllerGroup tcg = TunnelControllerGroup.getInstance(I2PAppContext.getGlobalContext());
+                if (tcg == null)
+                    return;
                 TunnelConfig cfg = TunnelUtil.createConfigFromWizard(getActivity(), tcg, tunnelData);
                 TunnelEntry tunnel = TunnelEntry.createNewTunnel(getActivity(), tcg, cfg);
 
============================================================
--- app/src/main/java/net/i2p/android/i2ptunnel/preferences/BaseTunnelPreferenceFragment.java	92f69e9463cfd3ff49b444bc681f3bdc7076bf2c
+++ app/src/main/java/net/i2p/android/i2ptunnel/preferences/BaseTunnelPreferenceFragment.java	90125c0ebb3c33951c27bb482cc7768db4b75c28
@@ -7,6 +7,7 @@ import android.widget.Toast;
 import android.support.v7.preference.PreferenceScreen;
 import android.widget.Toast;
 
+import net.i2p.I2PAppContext;
 import net.i2p.android.i2ptunnel.util.SaveTunnelTask;
 import net.i2p.android.i2ptunnel.util.TunnelUtil;
 import net.i2p.android.preferences.util.CustomPreferenceFragment;
@@ -30,7 +31,7 @@ public abstract class BaseTunnelPreferen
     public void onCreatePreferences(Bundle paramBundle, String s) {
         String error;
         try {
-            mGroup = TunnelControllerGroup.getInstance();
+            mGroup = TunnelControllerGroup.getInstance(I2PAppContext.getGlobalContext());
             error = mGroup == null ? getResources().getString(R.string.i2ptunnel_not_initialized) : null;
         } catch (IllegalArgumentException iae) {
             mGroup = null;

comment:5 Changed 6 weeks ago by zzz

I'm not happy with the test results of my patch in comment 4 above, it's causing TCG to start before the router. Needs rework and further testing.

comment:6 Changed 6 weeks ago by zzz

Resolution: fixed
Status: needs_workclosed

Confirmed the comment 4 patch was completely wrong, sorry.
Tweaked the checked-in patch from comment 3, to catch the null TCG sooner, as it wasn't clear what might happen from passing a null tcg to TunnelUtil?.createConfigFromWizard()
In ecb9de0333e91f3a1e4cc31a9c609ffd25e4778c

Note: See TracTickets for help on using tickets.