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.

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 ebe1d0080041e031a412a0490e85c1a930594910..878a9dd73e28d269b7f6e8a018366043058b329a 100644 (file)
--- 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<Boolean> 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 372b25c55929b4da378645e439498357b583ce1f..6380712c9458050654847bf00e5b7adcc894d63c 100644 (file)
--- 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,