Fix startSendFeedback crash

- mPsiphonTunnel is null

- For getNetworkID, use isVpnMode = false

MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -429,7 +429,37 @@ public class PsiphonTunnel {
 
                                     @Override
                                     public String getNetworkID() {
-                                        return PsiphonTunnel.getNetworkID(context, mPsiphonTunnel.isVpnMode());
+
+                                        // startSendFeedback is invoked from the Psiphon UI process, not the Psiphon
+                                        // VPN process.
+                                        //
+                                        // Case 1: no VPN is running
+                                        //
+                                        // isVpnMode = true/false doesn't change the network ID; the network ID will
+                                        // be the physical network ID, and feedback may load load tactics and may
+                                        // fetch tactics.
+                                        //
+                                        // Case 2: Psiphon VPN is running
+                                        //
+                                        // In principle, we might want to set isVpnMode = true so that we obtain the
+                                        // physical network ID and load any existing tactics. However, as the VPN
+                                        // holds a lock on the data store, the load will fail; also no tactics request
+                                        // is attempted.
+                                        //
+                                        // Hypothetically, if a tactics request did proceed, the tunneled client GeoIP
+                                        // would not reflect the actual client location, and so it's safer to set
+                                        // isVpnMode = false to ensure fetched tactics are stored under a distinct
+                                        // Network ID ("VPN").
+                                        //
+                                        // Case 3: another VPN is running
+                                        //
+                                        // Unlike case 2, there's no Psiphon VPN process holding the data store lock.
+                                        // As with case 2, there's some merit to setting isVpnMode = true in order to
+                                        // load existing tactics, but since a tactics request may proceed, it's safer
+                                        // to set isVpnMode = false and store fetched tactics under a distinct
+                                        // Network ID ("VPN").
+
+                                        return PsiphonTunnel.getNetworkID(context, false);
                                     }
 
                                     @Override

psiphon/feedback.go

@@ -121,8 +121,23 @@ func SendFeedback(ctx context.Context, config *Config, diagnostics, uploadPath s
 	p.Close()
 	getTacticsCtx, cancelFunc := context.WithTimeout(ctx, timeout)
 	defer cancelFunc()
-	// Note: GetTactics will fail silently if the datastore used for retrieving
-	// and storing tactics is opened by another process.
+
+	// Limitation: GetTactics will fail silently if the datastore used for
+	// retrieving and storing tactics is opened by another process. This can
+	// be the case on Android an iOS where SendFeedback is invoked by the UI
+	// process while tunneling is run by the VPN process.
+	//
+	// - When the Psiphon VPN is running, GetTactics won't load tactics.
+	//   However, tactics may be less critical since feedback will be
+	//   tunneled. This outcome also avoids fetching tactics while tunneled,
+	//   where otherwise the client GeoIP used for tactics would reflect the
+	//   tunnel egress point.
+	//
+	// - When the Psiphon VPN is not running, this will load tactics, and
+	//   potentially fetch tactics, with either the correct, untunneled GeoIP
+	//   or a network ID of "VPN" if some other non-Psiphon VPN is running
+	//   (the caller should ensure a network ID of "VPN" in this case).
+
 	GetTactics(getTacticsCtx, config)
 
 	// Get the latest client parameters