android: Send network change events from a separate thread via JNI
authorTobias Brunner <tobias@strongswan.org>
Wed, 15 Feb 2017 15:08:35 +0000 (16:08 +0100)
committerTobias Brunner <tobias@strongswan.org>
Fri, 17 Feb 2017 12:07:30 +0000 (13:07 +0100)
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.

src/frontends/android/app/src/main/java/org/strongswan/android/logic/NetworkManager.java
src/frontends/android/app/src/main/jni/libandroidbridge/kernel/network_manager.c

index ebe1d00..878a9dd 100644 (file)
@@ -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);
+               }
        }
 
        /**
index 372b25c..6380712 100644 (file)
@@ -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,