android: don't set vpnService to nil when state is Stopped

android/src/main/java/com/tailscale/ipn/IPNService.kt

@@ -73,10 +73,14 @@ open class IPNService : VpnService(), libtailscale.IPNService {
   override fun close() {
     app.setWantRunning(false) { updateVpnStatus(false) }
     Notifier.setState(Ipn.State.Stopping)
-    stopForeground(STOP_FOREGROUND_REMOVE)
+    disconnectVPN()
     Libtailscale.serviceDisconnect(this)
   }
 
+  override fun disconnectVPN(){
+    stopSelf()
+  }
+
   override fun onDestroy() {
     close()
     super.onDestroy()

libtailscale/backend.go

@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime/debug"
 	"sync"
 	"sync/atomic"
@@ -165,36 +166,24 @@ func (a *App) runBackend(ctx context.Context) error {
 		select {
 		case s := <-stateCh:
 			state = s
-			if cfg.rcfg != nil && state >= ipn.Starting && vpnService.service != nil {
+			if state >= ipn.Starting && vpnService.service != nil && b.isConfigNonNilAndDifferent(cfg.rcfg, cfg.dcfg) {
 				// On state change, check if there are router or config changes requiring an update to VPNBuilder
-				if err := b.updateTUN(vpnService.service, cfg.rcfg, cfg.dcfg); err != nil {
+				if err := b.updateTUN(cfg.rcfg, cfg.dcfg); err != nil {
 					if errors.Is(err, errMultipleUsers) {
 						// TODO: surface error to user
 					}
-					log.Printf("VPN update failed: %v", err)
-
-					mp := new(ipn.MaskedPrefs)
-					mp.WantRunning = false
-					mp.WantRunningSet = true
-
-					_, err := a.EditPrefs(*mp)
-					if err != nil {
-						log.Printf("localapi edit prefs error %v", err)
-					}
-
-					b.lastCfg = nil
-					b.CloseTUNs()
+					a.closeVpnService(err, b)
 				}
 			}
 		case n := <-netmapCh:
 			networkMap = n
 		case c := <-configs:
 			cfg = c
-			if b == nil || vpnService.service == nil || cfg.rcfg == nil {
+			if vpnService.service == nil || !b.isConfigNonNilAndDifferent(cfg.rcfg, cfg.dcfg) {
 				configErrs <- nil
 				break
 			}
-			configErrs <- b.updateTUN(vpnService.service, cfg.rcfg, cfg.dcfg)
+			configErrs <- b.updateTUN(cfg.rcfg, cfg.dcfg)
 		case s := <-onVPNRequested:
 			if vpnService.service != nil && vpnService.service.ID() == s.ID() {
 				// Still the same VPN instance, do nothing
@@ -230,12 +219,9 @@ func (a *App) runBackend(ctx context.Context) error {
 			if networkMap != nil {
 				// TODO
 			}
-			if cfg.rcfg != nil && state >= ipn.Starting {
-				if err := b.updateTUN(vpnService.service, cfg.rcfg, cfg.dcfg); err != nil {
-					log.Printf("VPN update failed: %v", err)
-					vpnService.service.Close()
-					b.lastCfg = nil
-					b.CloseTUNs()
+			if state >= ipn.Starting && b.isConfigNonNilAndDifferent(cfg.rcfg, cfg.dcfg) {
+				if err := b.updateTUN(cfg.rcfg, cfg.dcfg); err != nil {
+					a.closeVpnService(err, b)
 				}
 			}
 		case s := <-onDisconnect:
@@ -245,9 +231,7 @@ func (a *App) runBackend(ctx context.Context) error {
 				vpnService.service = nil
 			}
 		case i := <-onDNSConfigChanged:
-			if b != nil {
-				go b.NetworkChanged(i)
-			}
+			go b.NetworkChanged(i)
 		}
 	}
 }
@@ -346,3 +330,29 @@ func (a *App) newBackend(dataDir, directFileRoot string, appCtx AppContext, stor
 	}()
 	return b, nil
 }
+
+func (b *backend) isConfigNonNilAndDifferent(rcfg *router.Config, dcfg *dns.OSConfig) bool {
+	if reflect.DeepEqual(rcfg, b.lastCfg) && reflect.DeepEqual(dcfg, b.lastDNSCfg) {
+		b.logger.Logf("isConfigNonNilAndDifferent: no change to Routes or DNS, ignore")
+		return false
+	}
+	return rcfg != nil
+}
+
+func (a *App) closeVpnService(err error, b *backend) {
+	log.Printf("VPN update failed: %v", err)
+
+	mp := new(ipn.MaskedPrefs)
+	mp.WantRunning = false
+	mp.WantRunningSet = true
+
+	if _, localApiErr := a.EditPrefs(*mp); localApiErr != nil {
+		log.Printf("localapi edit prefs error %v", localApiErr)
+	}
+
+	b.lastCfg = nil
+	b.CloseTUNs()
+
+	vpnService.service.DisconnectVPN()
+	vpnService.service = nil
+}

libtailscale/interfaces.go

@@ -80,6 +80,8 @@ type IPNService interface {
 
 	Close()
 
+	DisconnectVPN()
+
 	UpdateVpnStatus(bool)
 }
 

libtailscale/net.go

@@ -9,7 +9,6 @@ import (
 	"log"
 	"net"
 	"net/netip"
-	"reflect"
 	"runtime/debug"
 	"strings"
 	"syscall"
@@ -125,12 +124,7 @@ var googleDNSServers = []netip.Addr{
 	netip.MustParseAddr("2001:4860:4860::8844"),
 }
 
-func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.OSConfig) error {
-	if reflect.DeepEqual(rcfg, b.lastCfg) && reflect.DeepEqual(dcfg, b.lastDNSCfg) {
-		b.logger.Logf("updateTUN: no change to Routes or DNS, ignore")
-		return nil
-	}
-
+func (b *backend) updateTUN(rcfg *router.Config, dcfg *dns.OSConfig) error {
 	b.logger.Logf("updateTUN: changed")
 	defer b.logger.Logf("updateTUN: finished")
 
@@ -147,7 +141,6 @@ func (b *backend) updateTUN(service IPNService, rcfg *router.Config, dcfg *dns.O
 	if len(rcfg.LocalAddrs) == 0 {
 		return nil
 	}
-	vpnService.service = service
 	builder := vpnService.service.NewBuilder()
 	b.logger.Logf("updateTUN: got new builder")
 
@@ -258,7 +251,6 @@ func closeFileDescriptor() error {
 func (b *backend) CloseTUNs() {
 	b.lastCfg = nil
 	b.devices.Shutdown()
-	vpnService.service = nil
 }
 
 // ifname is the interface name retrieved from LinkProperties on network change. If a network is lost, an empty string is passed in.