Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion netbird
Submodule netbird updated 132 files
29 changes: 29 additions & 0 deletions tool/src/main/java/io/netbird/client/tool/EngineRestarter.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.util.Log;

import io.netbird.client.tool.networks.NetworkToggleListener;
import io.netbird.gomobile.android.ConnectionListener;

/**
* <p>EngineRestarter restarts the Go engine.</p>
Expand Down Expand Up @@ -52,6 +53,7 @@ private void restartEngine() {
if (isRestartInProgress) {
Log.e(LOGTAG, "engine restart timeout - forcing flag reset");
isRestartInProgress = false;
notifyDisconnected();
}
};

Expand All @@ -72,6 +74,7 @@ public void onStarted() {
@Override
public void onStopped() {
Log.d(LOGTAG, "engine is stopped, restarting...");
notifyConnecting();
engineRunner.runWithoutAuth();
}

Expand All @@ -81,6 +84,7 @@ public void onError(String msg) {
isRestartInProgress = false; // Resetting flag on error as well
handler.removeCallbacks(timeoutCallback); // Cancel timeout
engineRunner.removeServiceStateListener(this);
notifyDisconnected();
}
};
currentListener = serviceStateListener;
Expand All @@ -94,9 +98,34 @@ public void onError(String msg) {
}

Log.d(LOGTAG, "engine is running, stopping due to network change");
notifyConnecting();
engineRunner.stop();
}

private void notifyConnecting() {
ConnectionListener listener = engineRunner.getConnectionListener();
if (listener == null) {
return;
}
try {
listener.onConnecting();
} catch (Exception e) {
Log.w(LOGTAG, "onConnecting notification failed: " + e.getMessage());
}
}

private void notifyDisconnected() {
ConnectionListener listener = engineRunner.getConnectionListener();
if (listener == null) {
return;
}
try {
listener.onDisconnected();
} catch (Exception e) {
Log.w(LOGTAG, "onDisconnected notification failed: " + e.getMessage());
}
}

@Override
public void onNetworkTypeChanged() {
Log.d(LOGTAG, "network type changed, scheduling restart with "
Expand Down
7 changes: 7 additions & 0 deletions tool/src/main/java/io/netbird/client/tool/EngineRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class EngineRunner {
private boolean engineIsRunning = false;
Set<ServiceStateListener> serviceStateListeners = ConcurrentHashMap.newKeySet();
private final Client goClient;
private ConnectionListener connectionListener;

public EngineRunner(Context context, NetworkChangeListener networkChangeListener, TunAdapter tunAdapter,
IFaceDiscover iFaceDiscover, String versionName, boolean isTraceLogEnabled, boolean isDebuggable,
Expand Down Expand Up @@ -124,13 +125,19 @@ public synchronized boolean isRunning() {
}

public synchronized void setConnectionListener(ConnectionListener listener) {
this.connectionListener = listener;
goClient.setConnectionListener(listener);
}

public synchronized void removeStatusListener() {
this.connectionListener = null;
goClient.removeConnectionListener();
}

synchronized ConnectionListener getConnectionListener() {
return connectionListener;
}

public synchronized void addServiceStateListener(ServiceStateListener serviceStateListener) {
if (engineIsRunning) {
serviceStateListener.onStarted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ public void startForeground() {
NotificationChannel channel = new NotificationChannel(
channelId,
service.getResources().getString(R.string.fg_notification_channel_name),
NotificationManager.IMPORTANCE_DEFAULT);
NotificationManager.IMPORTANCE_LOW);
channel.setSound(null, null);
channel.enableVibration(false);
((NotificationManager) service.getSystemService(Context.NOTIFICATION_SERVICE)).createNotificationChannel(channel);

Intent notificationIntent = new Intent();
Expand Down
9 changes: 5 additions & 4 deletions tool/src/main/java/io/netbird/client/tool/VPNService.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,15 @@ public void onCreate() {
// Create foreground notification before initializing engine
fgNotification = new ForegroundNotification(this);

// Create network availability listener before initializing engine
networkAvailabilityListener = new ConcreteNetworkAvailabilityListener();


engineRunner = new EngineRunner(this, notifier, tunAdapter, iFaceDiscover, versionName,
preferences.isTraceLogEnabled(), Version.isDebuggable(this), profileManager);
engineRunner.addServiceStateListener(serviceStateListener);

// Create network availability listener after the engine runner so we
// can gate notifications on the engine actually being up; this avoids
// acting on Android's initial onAvailable burst during cold start.
networkAvailabilityListener = new ConcreteNetworkAvailabilityListener(engineRunner::isRunning);

engineRestarter = new EngineRestarter(engineRunner);
networkAvailabilityListener.subscribe(engineRestarter);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,24 @@

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BooleanSupplier;

public class ConcreteNetworkAvailabilityListener implements NetworkAvailabilityListener {
private final Map<Integer, Boolean> availableNetworkTypes;
private final BooleanSupplier shouldNotify;
private NetworkToggleListener listener;

public ConcreteNetworkAvailabilityListener() {
this(() -> true);
}

// shouldNotify is consulted before each listener notification. Pass
// engineRunner::isRunning to swallow the initial onAvailable burst that
// fires right after registerNetworkCallback; until the engine is actually
// running there is nothing to restart.
public ConcreteNetworkAvailabilityListener(BooleanSupplier shouldNotify) {
this.availableNetworkTypes = new ConcurrentHashMap<>();
this.shouldNotify = shouldNotify;
}

@Override
Expand Down Expand Up @@ -38,9 +49,14 @@ public void onNetworkLost(@Constants.NetworkType int networkType) {
}

private void notifyListener() {
if (listener != null) {
listener.onNetworkTypeChanged();
NetworkToggleListener l = listener;
if (l == null) {
return;
}
if (!shouldNotify.getAsBoolean()) {
return;
}
l.onNetworkTypeChanged();
}

public void subscribe(NetworkToggleListener listener) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ public class NetworkChangeDetector {
private static final String LOGTAG = NetworkChangeDetector.class.getSimpleName();
private final ConnectivityManager connectivityManager;
private ConnectivityManager.NetworkCallback networkCallback;
private ConnectivityManager.NetworkCallback defaultNetworkCallback;
private volatile NetworkAvailabilityListener listener;

public NetworkChangeDetector(ConnectivityManager connectivityManager) {
this.connectivityManager = connectivityManager;
initNetworkCallback();
initDefaultNetworkCallback();
}

private void checkNetworkCapabilities(Network network, Consumer<Integer> operation) {
Expand Down Expand Up @@ -58,10 +60,37 @@ public void onCapabilitiesChanged(@NonNull Network network, @NonNull NetworkCapa
};
}

private void initDefaultNetworkCallback() {
defaultNetworkCallback = new ConnectivityManager.NetworkCallback() {
@Override
public void onAvailable(@NonNull Network network) {
Log.d(LOGTAG, "default network became " + network + ", binding process to it");
try {
if (!connectivityManager.bindProcessToNetwork(network)) {
Log.w(LOGTAG, "bindProcessToNetwork returned false for " + network);
}
} catch (Exception e) {
Log.e(LOGTAG, "bindProcessToNetwork failed", e);
}
}

@Override
public void onLost(@NonNull Network network) {
Log.d(LOGTAG, "default network " + network + " lost, clearing process binding");
try {
connectivityManager.bindProcessToNetwork(null);
} catch (Exception e) {
Log.e(LOGTAG, "bindProcessToNetwork(null) failed", e);
}
}
Comment on lines +63 to +85
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard process binding with active/current-network state.

onLost() always clears the process binding, so an old default network’s late onLost() can erase a newer binding from onAvailable(). Similarly, an in-flight onAvailable() during unregisterNetworkCallback() can re-bind the process after shutdown. Track the currently bound default network and ignore callbacks once unregister starts.

🔧 Proposed direction
+    private final Object defaultNetworkBindingLock = new Object();
+    private boolean defaultNetworkCallbackActive = false;
+    private Network boundDefaultNetwork;

     private void initDefaultNetworkCallback() {
         defaultNetworkCallback = new ConnectivityManager.NetworkCallback() {
             `@Override`
             public void onAvailable(`@NonNull` Network network) {
+                synchronized (defaultNetworkBindingLock) {
+                    if (!defaultNetworkCallbackActive) {
+                        return;
+                    }
+                    boundDefaultNetwork = network;
+                }
                 Log.d(LOGTAG, "default network became " + network + ", binding process to it");
                 try {
                     if (!connectivityManager.bindProcessToNetwork(network)) {
                         Log.w(LOGTAG, "bindProcessToNetwork returned false for " + network);
@@
             `@Override`
             public void onLost(`@NonNull` Network network) {
+                synchronized (defaultNetworkBindingLock) {
+                    if (!defaultNetworkCallbackActive || !network.equals(boundDefaultNetwork)) {
+                        return;
+                    }
+                    boundDefaultNetwork = null;
+                }
                 Log.d(LOGTAG, "default network " + network + " lost, clearing process binding");
                 try {
                     connectivityManager.bindProcessToNetwork(null);
                 } catch (Exception e) {
@@
     public void unregisterNetworkCallback() {
+        synchronized (defaultNetworkBindingLock) {
+            defaultNetworkCallbackActive = false;
+            boundDefaultNetwork = null;
+        }
         try {
             connectivityManager.unregisterNetworkCallback(networkCallback);

Also set defaultNetworkCallbackActive = true before registering the default callback, and reset it if registration fails.

Also applies to: 96-111

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@tool/src/main/java/io/netbird/client/tool/networks/NetworkChangeDetector.java`
around lines 63 - 85, The onAvailable/onLost handlers in
initDefaultNetworkCallback must be guarded by an atomic/current bound-network
state and a callback-active flag so late onLost or onAvailable after unregister
don't undo a newer binding; add a field (e.g., defaultNetworkCallbackActive) set
to true before registering the defaultNetworkCallback and set to false when
unregisterNetworkCallback begins, track the currentlyBoundDefaultNetwork (or
similar) when bindProcessToNetwork(network) succeeds, and in onLost only clear
the binding if the lost network equals the currentlyBoundDefaultNetwork and
defaultNetworkCallbackActive is true; likewise ignore onAvailable if
defaultNetworkCallbackActive is false to avoid rebinding after shutdown.

};
}

public void registerNetworkCallback() {
NetworkRequest.Builder builder = new NetworkRequest.Builder();
builder.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
connectivityManager.registerNetworkCallback(builder.build(), networkCallback);
connectivityManager.registerDefaultNetworkCallback(defaultNetworkCallback);
}

public void unregisterNetworkCallback() {
Expand All @@ -70,6 +99,16 @@ public void unregisterNetworkCallback() {
} catch (Exception e) {
Log.e(LOGTAG, "failed to unregister network callback", e);
}
try {
connectivityManager.unregisterNetworkCallback(defaultNetworkCallback);
} catch (Exception e) {
Log.e(LOGTAG, "failed to unregister default network callback", e);
}
try {
connectivityManager.bindProcessToNetwork(null);
} catch (Exception e) {
Log.e(LOGTAG, "bindProcessToNetwork(null) on unregister failed", e);
}
}

public void subscribe(NetworkAvailabilityListener listener) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void deactivateMobile() {
this.listener.onNetworkLost(Constants.NetworkType.MOBILE);
}
}

private static class MockNetworkToggleListener implements NetworkToggleListener {
private int totalTimesNetworkTypeChanged = 0;

Expand All @@ -47,7 +47,7 @@ public void resetCounter() {
public void shouldNotifyListenerNetworkUpgraded() {
// Assemble:
var networkToggleListener = new MockNetworkToggleListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener(() -> true);
networkAvailabilityListener.subscribe(networkToggleListener);

var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);
Expand All @@ -64,7 +64,7 @@ public void shouldNotifyListenerNetworkUpgraded() {
public void shouldNotifyListenerNetworkDowngraded() {
// Assemble:
var networkToggleListener = new MockNetworkToggleListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener(() -> true);
networkAvailabilityListener.subscribe(networkToggleListener);

var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);
Expand All @@ -82,7 +82,7 @@ public void shouldNotifyListenerNetworkDowngraded() {
public void shouldNotNotifyListenerNetworkDidNotUpgrade() {
// Assemble:
var networkToggleListener = new MockNetworkToggleListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener(() -> true);
networkAvailabilityListener.subscribe(networkToggleListener);

var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);
Expand All @@ -103,7 +103,7 @@ public void shouldNotNotifyListenerNetworkDidNotUpgrade() {
public void shouldNotNotifyListenerNoNetworksAvailable() {
// Assemble:
var networkToggleListener = new MockNetworkToggleListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener(() -> true);
networkAvailabilityListener.subscribe(networkToggleListener);

var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);
Expand All @@ -118,4 +118,23 @@ public void shouldNotNotifyListenerNoNetworksAvailable() {
// Assert:
assertEquals(0, networkToggleListener.totalTimesNetworkTypeChanged);
}

@Test
public void shouldNotNotifyListenerWhenEngineNotRunning() {
// Assemble: engine never running, so initial onAvailable burst from
// Android must not trigger a restart.
var networkToggleListener = new MockNetworkToggleListener();
var networkAvailabilityListener = new ConcreteNetworkAvailabilityListener(() -> false);
networkAvailabilityListener.subscribe(networkToggleListener);

var networkChangeDetector = new MockNetworkChangeDetector(networkAvailabilityListener);

// Act:
networkChangeDetector.activateMobile();
networkChangeDetector.activateWifi();
networkChangeDetector.deactivateWifi();

// Assert:
assertEquals(0, networkToggleListener.totalTimesNetworkTypeChanged);
}
}
Loading