From 94375d46dc71aa61e408cbcc976e3b097ba553b4 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 15 Feb 2017 16:08:35 +0100 Subject: [PATCH] android: Send network change events from a separate thread via JNI Doing this from the main UI thread (which delivers the broadcast) might cause an ANR if there is a delay (e.g. while acquiring a mutex in the native parts). There might also have been a race condition during termination previously because Unregister() was not synchronized so there might have been dangling events that got delivered while or after the mutex in the native parts was destroyed. --- .../android/logic/NetworkManager.java | 63 ++++++++++++++++++- .../libandroidbridge/kernel/network_manager.c | 9 ++- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/NetworkManager.java b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/NetworkManager.java index ebe1d0080..878a9dd73 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/NetworkManager.java +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/NetworkManager.java @@ -22,10 +22,14 @@ import android.content.IntentFilter; import android.net.ConnectivityManager; import android.net.NetworkInfo; -public class NetworkManager extends BroadcastReceiver +import java.util.LinkedList; + +public class NetworkManager extends BroadcastReceiver implements Runnable { private final Context mContext; - private boolean mRegistered; + private volatile boolean mRegistered; + private Thread mEventNotifier; + private LinkedList mEvents = new LinkedList<>(); public NetworkManager(Context context) { @@ -34,12 +38,30 @@ public class NetworkManager extends BroadcastReceiver public void Register() { + mEvents.clear(); + mRegistered = true; + mEventNotifier = new Thread(this); + mEventNotifier.start(); mContext.registerReceiver(this, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION)); } public void Unregister() { mContext.unregisterReceiver(this); + mRegistered = false; + synchronized (this) + { + notifyAll(); + } + try + { + mEventNotifier.join(); + mEventNotifier = null; + } + catch (InterruptedException e) + { + e.printStackTrace(); + } } public boolean isConnected() @@ -56,7 +78,42 @@ public class NetworkManager extends BroadcastReceiver @Override public void onReceive(Context context, Intent intent) { - networkChanged(!isConnected()); + synchronized (this) + { + mEvents.addLast(isConnected()); + notifyAll(); + } + } + + @Override + public void run() + { + while (mRegistered) + { + boolean connected; + + synchronized (this) + { + try + { + while (mRegistered && mEvents.isEmpty()) + { + wait(); + } + } + catch (InterruptedException ex) + { + break; + } + if (!mRegistered) + { + break; + } + connected = mEvents.removeFirst(); + } + /* call the native parts without holding the lock */ + networkChanged(!connected); + } } /** diff --git a/src/frontends/android/app/src/main/jni/libandroidbridge/kernel/network_manager.c b/src/frontends/android/app/src/main/jni/libandroidbridge/kernel/network_manager.c index 372b25c55..6380712c9 100644 --- a/src/frontends/android/app/src/main/jni/libandroidbridge/kernel/network_manager.c +++ b/src/frontends/android/app/src/main/jni/libandroidbridge/kernel/network_manager.c @@ -123,13 +123,20 @@ static void unregister_network_manager(private_network_manager_t *this) METHOD(network_manager_t, remove_connectivity_cb, void, private_network_manager_t *this, connectivity_cb_t cb) { + bool unregister = FALSE; + this->mutex->lock(this->mutex); if (this->connectivity_cb.cb == cb) { this->connectivity_cb.cb = NULL; - unregister_network_manager(this); + unregister = TRUE; } this->mutex->unlock(this->mutex); + if (unregister) + { /* this call blocks until a possible networkChanged call returned so + * we can't hold the mutex */ + unregister_network_manager(this); + } } METHOD(network_manager_t, is_connected, bool,