From 358dcb94b25f43fd4d5cad7bbab4da85deab87c8 Mon Sep 17 00:00:00 2001 From: Denozordec Date: Sat, 10 Oct 2026 09:09:39 +0700 Subject: [PATCH] android: CharonVpnService - safer foreground and teardown handling - DISCONNECT (plain startService) no longer promotes to the foreground, and startForeground() refusals on Android 12+ are logged, not fatal - drop the placeholder notification when nothing was running, so the service does not stay in the foreground with a stale Disabled state - onDestroy: shutdownNow/await the profile executor before closing the database; profile loads catch errors instead of killing the process - unbind by bind result, not by mService (binding leaked if destroyed before onServiceConnected) - the connection thread's crash guard can no longer throw itself Co-Authored-By: Claude Opus 5.5 --- .../android/logic/CharonVpnService.java | 93 +++++++++++++++++-- 1 file changed, 83 insertions(+), 10 deletions(-) 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 9e3c88e90..2592651a6 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 @@ -94,6 +94,7 @@ import java.util.SortedSet; import java.util.TreeSet; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import androidx.core.app.NotificationCompat; import androidx.core.content.ContextCompat; @@ -169,6 +170,7 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe private UnderlyingNetworkTracker mUnderlyingTracker; private Handler mHandler; private VpnStateService mService; + private boolean mBound; private final Object mServiceLock = new Object(); private final ServiceConnection mServiceConnection = new ServiceConnection() { @@ -217,8 +219,14 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe * first millisecond, and it avoids the system killing a fresh process * when the profile fails to load before addNotification() runs. The * state-specific notification replaces this one (via stateChanged()). */ - mShowNotification = true; - startForeground(VPN_STATE_NOTIFICATION_ID, buildNotification(false)); + boolean disconnect = intent != null && DISCONNECT_ACTION.equals(intent.getAction()); + if (!disconnect) + { /* DISCONNECT arrives via plain startService(): promoting to the + * foreground from the background is not allowed there (Android 12+) + * and would crash, and there is nothing to keep alive anyway */ + mShowNotification = true; + startForegroundSafe(); + } if (intent != null) { if (VPN_SERVICE_ACTION.equals(intent.getAction())) @@ -282,7 +290,16 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe { return null; } - return mDataSource.getVpnProfile(uuid); + try + { + return mDataSource.getVpnProfile(uuid); + } + catch (RuntimeException e) + { /* e.g. the service is being destroyed and closed the database; + * an uncaught exception on the executor would kill the process */ + DiagLog.i(TAG, "loading profile failed: " + e); + return null; + } } /** @@ -391,8 +408,8 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe /* use a separate thread as main thread for charon */ mConnectionHandler = new Thread(this); /* the thread is started when the service is bound */ - bindService(new Intent(this, VpnStateService.class), - mServiceConnection, Service.BIND_AUTO_CREATE); + mBound = bindService(new Intent(this, VpnStateService.class), + mServiceConnection, Service.BIND_AUTO_CREATE); createNotificationChannel(); } @@ -418,12 +435,28 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe { e.printStackTrace(); } - if (mService != null) + synchronized (mServiceLock) { - mService.unregisterListener(this); + if (mService != null) + { + mService.unregisterListener(this); + } + } + if (mBound) + { /* unbind even if onServiceConnected() never ran, or the binding leaks */ unbindService(mServiceConnection); + mBound = false; + } + /* queued profile loads must not run against the closed database */ + mProfileExecutor.shutdownNow(); + try + { + mProfileExecutor.awaitTermination(2, TimeUnit.SECONDS); + } + catch (InterruptedException e) + { + Thread.currentThread().interrupt(); } - mProfileExecutor.shutdown(); mDataSource.close(); } @@ -461,6 +494,11 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe if (mNextProfile == null) { setState(State.DISABLED); + /* nothing was running (e.g. DISCONNECT while already + * disconnected, or the profile failed to load): drop the + * placeholder notification so the service is not kept + * in the foreground with a stale "Disabled" state */ + removeNotification(); if (mTerminate) { break; @@ -537,7 +575,25 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe * and let the regular retry machinery handle it. */ Log.e(TAG, "Exception in the connection thread", ex); DiagLog.i(TAG, "exception in connection thread: " + ex); - stopCurrentConnection(); + try + { + stopCurrentConnection(); + } + catch (Exception again) + { /* the exception may have come from stopCurrentConnection() + * itself (e.g. a broken next profile): drop that profile + * so this guard cannot throw again and kill the thread */ + DiagLog.i(TAG, "cleanup after exception failed: " + again); + mNextProfile = null; + if (mCurrentProfile != null) + { + SimpleFetcher.disable(); + deinitializeCharon(); + sUserspacePushActive = false; + } + removeNotification(); + mBuilderAdapter.closeBlocking(); + } setError(ErrorState.GENERIC_ERROR); setState(State.DISABLED); mCurrentProfile = null; @@ -593,11 +649,28 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe public void run() { mShowNotification = true; - startForeground(VPN_STATE_NOTIFICATION_ID, buildNotification(false)); + startForegroundSafe(); } }); } + /** + * Promote the service to the foreground. Never throws: on Android 12+ the + * system may refuse the promotion while the app is in the background, which + * must not crash the process. + */ + private void startForegroundSafe() + { + try + { + startForeground(VPN_STATE_NOTIFICATION_ID, buildNotification(false)); + } + catch (RuntimeException e) + { + DiagLog.i(TAG, "startForeground refused: " + e); + } + } + /** * Remove the permanent notification. */