diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java index 2592651a6..a43094ee0 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java +++ b/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) - { - update(network); + { /* use the capabilities passed in: querying them again is an IPC + * 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:"); - for (Network underlying : current) - { - handles.append(" #").append(underlying.getNetworkHandle()); - } - DiagLog.i(TAG, handles.toString()); + handles.append(" #").append(underlying.getNetworkHandle()); } - if (mCurrentProfile != null && - Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1) - { /* reflect roaming in the system's VPN routing right away - * instead of waiting for the next establish(). Passing - * null falls back to the system default, matching the - * establish-time behavior; the call is a no-op for the - * system if no tunnel is currently up. */ - setUnderlyingNetworks(current.length > 0 ? current : null); - } - /* let the state service react to the change (e.g. retry - * immediately instead of waiting out a backoff that was sized - * for the previous network) */ - synchronized (mServiceLock) + DiagLog.i(TAG, handles.toString()); + } + if (mCurrentProfile != null && + Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1) + { /* reflect roaming in the system's VPN routing right away + * instead of waiting for the next establish(). Passing + * null falls back to the system default, matching the + * establish-time behavior; the call is a no-op for the + * system if no tunnel is currently up. */ + setUnderlyingNetworks(current.length > 0 ? current : null); + } + /* let the state service react to the change (e.g. retry + * 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 - * repeat the call via setUnderlyingNetworks() */ - mUnderlyingTracker.mApplied = networks; + return 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) {