android: CharonVpnService - handover race, cheaper network/notification updates

- underlying-network bookkeeping is guarded by one lock and re-applied
  after establish(), so a handover racing with it no longer leaves the
  tunnel attributed to a lost network
- use the capabilities passed to onCapabilitiesChanged (no IPC per
  signal-strength update); mCurrentProfile is volatile
- countdown-only notification re-posts are limited to one per 5 s
- the connected chronometer keeps its start time across re-posts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java

@@ -158,7 +158,8 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 	/* profile lookups are SQLite queries and must not run on the main thread */
 	private final ExecutorService mProfileExecutor = Executors.newSingleThreadExecutor();
 	private Thread mConnectionHandler;
-	private VpnProfile mCurrentProfile;
+	/* read by the ConnectivityThread (UnderlyingNetworkTracker) too */
+	private volatile VpnProfile mCurrentProfile;
 	private volatile String mCurrentCertificateAlias;
 	private volatile String mCurrentUserCertificateAlias;
 	private VpnProfile mNextProfile;
@@ -166,6 +167,12 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 	private volatile boolean mTerminate;
 	private volatile boolean mIsDisconnecting;
 	private volatile boolean mShowNotification;
+	/* start of the current CONNECTED session for the notification chronometer */
+	private volatile long mConnectedSince;
+	/* rate limit for countdown-only notification updates (main thread only) */
+	private static final long NOTIFICATION_MIN_INTERVAL_MS = 5000;
+	private String mLastNotifiedKey;
+	private long mLastNotifiedAt;
 	private final BuilderAdapter mBuilderAdapter = new BuilderAdapter();
 	private UnderlyingNetworkTracker mUnderlyingTracker;
 	private Handler mHandler;
@@ -742,6 +749,10 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 			.setVisibility(publicVersion ? NotificationCompat.VISIBILITY_PUBLIC
 										 : NotificationCompat.VISIBILITY_PRIVATE);
 		int s = R.string.state_disabled;
+		if (state != State.CONNECTED || error != ErrorState.NO_ERROR)
+		{
+			mConnectedSince = 0;
+		}
 		if (error != ErrorState.NO_ERROR)
 		{
 			s = mService.getErrorText();
@@ -788,6 +799,13 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 					s = R.string.state_connected;
 					builder.setColor(ContextCompat.getColor(this, R.color.success_text));
 					builder.setUsesChronometer(true);
+					/* without an explicit start every re-post would reset the
+					 * chronometer to 0:00 */
+					if (mConnectedSince == 0)
+					{
+						mConnectedSince = System.currentTimeMillis();
+					}
+					builder.setWhen(mConnectedSince);
 					add_action = true;
 					break;
 				case DISCONNECTING:
@@ -831,6 +849,17 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 	{
 		if (mShowNotification)
 		{
+			/* the retry countdown notifies every 1-5 s; each re-post builds two
+			 * notifications and several PendingIntents (IPC).  Countdown-only
+			 * updates are rate-limited, real state/error changes post at once */
+			String key = (mService != null ? mService.getState() + "/" + mService.getErrorState() : "");
+			long now = SystemClock.elapsedRealtime();
+			if (key.equals(mLastNotifiedKey) && now - mLastNotifiedAt < NOTIFICATION_MIN_INTERVAL_MS)
+			{
+				return;
+			}
+			mLastNotifiedKey = key;
+			mLastNotifiedAt = now;
 			NotificationManager manager = (NotificationManager)getSystemService(Context.NOTIFICATION_SERVICE);
 			manager.notify(VPN_STATE_NOTIFICATION_ID, buildNotification(false));
 		}
@@ -1664,7 +1693,9 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 		private ConnectivityManager mConnectivityManager;
 		private boolean mRegistered;
 		/* the set of networks last reported to the system, to skip redundant
-		 * setUnderlyingNetworks() calls (the callbacks can be chatty) */
+		 * setUnderlyingNetworks() calls (the callbacks can be chatty).
+		 * Guarded by mNetworks: written by the ConnectivityThread and by
+		 * charon's thread around establish() */
 		private Network[] mApplied;
 
 		private final ConnectivityManager.NetworkCallback mCallback = new ConnectivityManager.NetworkCallback()
@@ -1672,19 +1703,20 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 			@Override
 			public void onAvailable(Network network)
 			{
-				update(network);
+				update(network, mConnectivityManager.getNetworkCapabilities(network));
 			}
 
 			@Override
 			public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities)
-			{
+			{	/* use the capabilities passed in: querying them again is an IPC
-				update(network);
+				 * on every signal-strength update */
+				update(network, networkCapabilities);
 			}
 
 			@Override
 			public void onLost(Network network)
 			{
-				update(network);
+				update(network, null);
 			}
 		};
 
@@ -1734,8 +1766,8 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 			synchronized (mNetworks)
 			{
 				mNetworks.clear();
+				mApplied = null;
 			}
-			mApplied = null;
 		}
 
 		private boolean isUsableUnderlying(NetworkCapabilities capabilities)
@@ -1746,48 +1778,49 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 				   !capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN);
 		}
 
-		private void update(Network network)
+		private void update(Network network, NetworkCapabilities capabilities)
 		{
 			Network[] current;
 			synchronized (mNetworks)
 			{
 				mNetworks.remove(network);
-				if (isUsableUnderlying(mConnectivityManager.getNetworkCapabilities(network)))
+				if (isUsableUnderlying(capabilities))
 				{
 					mNetworks.add(network);
 				}
 				current = mNetworks.toArray(new Network[0]);
-			}
+				if (sameNetworks(current, mApplied))
-			if (!sameNetworks(current, mApplied))
+				{
-			{
+					return;
+				}
 				mApplied = current;
-				if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M)
+			}
+			if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M)
+			{
+				StringBuilder handles = new StringBuilder("underlying networks changed:");
+				for (Network underlying : current)
 				{
-					StringBuilder handles = new StringBuilder("underlying networks changed:");
+					handles.append(" #").append(underlying.getNetworkHandle());
-					for (Network underlying : current)
-					{
-						handles.append(" #").append(underlying.getNetworkHandle());
-					}
-					DiagLog.i(TAG, handles.toString());
 				}
-				if (mCurrentProfile != null &&
+				DiagLog.i(TAG, handles.toString());
-					Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1)
+			}
-				{	/* reflect roaming in the system's VPN routing right away
+			if (mCurrentProfile != null &&
-					 * instead of waiting for the next establish().  Passing
+				Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1)
-					 * null falls back to the system default, matching the
+			{	/* reflect roaming in the system's VPN routing right away
-					 * establish-time behavior; the call is a no-op for the
+				 * instead of waiting for the next establish().  Passing
-					 * system if no tunnel is currently up. */
+				 * null falls back to the system default, matching the
-					setUnderlyingNetworks(current.length > 0 ? current : null);
+				 * establish-time behavior; the call is a no-op for the
-				}
+				 * system if no tunnel is currently up. */
-				/* let the state service react to the change (e.g. retry
+				setUnderlyingNetworks(current.length > 0 ? current : null);
-				 * immediately instead of waiting out a backoff that was sized
+			}
-				 * for the previous network) */
+			/* let the state service react to the change (e.g. retry
-				synchronized (mServiceLock)
+			 * immediately instead of waiting out a backoff that was sized
+			 * for the previous network) */
+			synchronized (mServiceLock)
+			{
+				if (mService != null)
 				{
-					if (mService != null)
+					mService.onUnderlyingNetworksChanged();
-					{
-						mService.onUnderlyingNetworksChanged();
-					}
 				}
 			}
 		}
@@ -1837,24 +1870,61 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 				return mNetworks.isEmpty() ? null : mNetworks.toArray(new Network[0]);
 			}
 		}
+
+		/**
+		 * Snapshot for a builder that is about to establish(), remembered as
+		 * applied so the tracker doesn't immediately repeat it.
+		 */
+		Network[] snapshotForEstablish()
+		{
+			synchronized (mNetworks)
+			{
+				Network[] current = mNetworks.isEmpty() ? null : mNetworks.toArray(new Network[0]);
+				mApplied = current;
+				return current;
+			}
+		}
+
+		/**
+		 * After establish(): a network change that raced with it was applied
+		 * via setUnderlyingNetworks() before the tunnel existed (a no-op), so
+		 * the new tunnel may carry a stale set - re-apply the current one.
+		 */
+		void afterEstablish(Network[] used)
+		{
+			Network[] current;
+			synchronized (mNetworks)
+			{
+				current = mNetworks.toArray(new Network[0]);
+				if (sameNetworks(current, used))
+				{
+					return;
+				}
+				mApplied = current;
+			}
+			if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1)
+			{
+				DiagLog.i(TAG, "underlying networks changed during establish, re-applying");
+				setUnderlyingNetworks(current.length > 0 ? current : null);
+			}
+		}
 	}
 
 	/**
 	 * Report the physical networks below the tunnel to the builder (API 22+).
 	 */
-	private void applyUnderlyingNetworks(VpnService.Builder builder)
+	private Network[] applyUnderlyingNetworks(VpnService.Builder builder)
 	{
 		if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1)
 		{
-			Network[] networks = mUnderlyingTracker.getNetworks();
+			Network[] networks = mUnderlyingTracker.snapshotForEstablish();
 			if (networks != null)
 			{
 				builder.setUnderlyingNetworks(networks);
 			}
-			/* remember what was reported so the tracker doesn't immediately
+			return networks;
-			 * repeat the call via setUnderlyingNetworks() */
-			mUnderlyingTracker.mApplied = networks;
 		}
+		return null;
 	}
 
 	public class BuilderAdapter
@@ -1993,10 +2063,11 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 			try
 			{
 				mCache.applyData(mBuilder);
-				applyUnderlyingNetworks(mBuilder);
+				Network[] underlying = applyUnderlyingNetworks(mBuilder);
 				fd = mBuilder.establish();
 				if (fd != null)
 				{
+					mUnderlyingTracker.afterEstablish(underlying);
 					closeBlocking();
 				}
 			}
@@ -2053,8 +2124,12 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
 			{
 				Builder builder = createBuilder(mProfile.getName());
 				mEstablishedCache.applyData(builder, false);
-				applyUnderlyingNetworks(builder);
+				Network[] underlying = applyUnderlyingNetworks(builder);
 				fd = builder.establish();
+				if (fd != null)
+				{
+					mUnderlyingTracker.afterEstablish(underlying);
+				}
 			}
 			catch (Exception ex)
 			{