From 203043429b2c9c2f60dbf5a66177c0b80830d5eb Mon Sep 17 00:00:00 2001 From: cyBerta Date: Thu, 8 Mar 2018 01:42:01 +0100 Subject: #8876 introduce blocking OpenvpnServiceConnection for EIP and cancel connections more rigid --- .../java/se/leap/bitmaskclient/ConfigHelper.java | 8 ++ .../java/se/leap/bitmaskclient/EipFragment.java | 19 +--- .../main/java/se/leap/bitmaskclient/eip/EIP.java | 119 +++++++++++++-------- 3 files changed, 87 insertions(+), 59 deletions(-) (limited to 'app/src') diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java index 03f8a881..bfc77261 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java +++ b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java @@ -18,6 +18,7 @@ package se.leap.bitmaskclient; import android.content.Context; import android.content.SharedPreferences; +import android.os.Looper; import android.preference.PreferenceManager; import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -451,4 +452,11 @@ public class ConfigHelper { return result; } + public static void ensureNotOnMainThread(@NonNull Context context) throws IllegalStateException{ + Looper looper = Looper.myLooper(); + if (looper != null && looper == context.getMainLooper()) { + throw new IllegalStateException( + "calling this from your main thread can lead to deadlock"); + } + } } diff --git a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java index 34120859..797452d9 100644 --- a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java @@ -281,24 +281,11 @@ public class EipFragment extends Fragment implements Observer { protected void stopEipIfPossible() { Context context = getContext(); - if (context != null) { - if (isOpenVpnRunningWithoutNetwork()) { - // TODO move to EIP - // TODO see stopEIP function - Bundle resultData = new Bundle(); - resultData.putString(EIP_REQUEST, EIP_ACTION_STOP); - Intent intentUpdate = new Intent(BROADCAST_EIP_EVENT); - intentUpdate.addCategory(Intent.CATEGORY_DEFAULT); - intentUpdate.putExtra(BROADCAST_RESULT_CODE, Activity.RESULT_OK); - intentUpdate.putExtra(BROADCAST_RESULT_KEY, resultData); - Log.d(TAG, "sending broadcast"); - LocalBroadcastManager.getInstance(getActivity()).sendBroadcast(intentUpdate); - } else { - EipCommand.stopVPN(getContext()); - } - } else { + if (context == null) { Log.e(TAG, "context is null when trying to stop EIP"); + return; } + EipCommand.stopVPN(context); } private void askPendingStartCancellation() { diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/EIP.java b/app/src/main/java/se/leap/bitmaskclient/eip/EIP.java index 17285548..1e553ac2 100644 --- a/app/src/main/java/se/leap/bitmaskclient/eip/EIP.java +++ b/app/src/main/java/se/leap/bitmaskclient/eip/EIP.java @@ -29,16 +29,19 @@ import android.os.RemoteException; import android.os.ResultReceiver; import android.support.annotation.Nullable; import android.support.annotation.StringRes; +import android.support.annotation.WorkerThread; import android.support.v4.content.LocalBroadcastManager; import android.util.Log; import org.json.JSONException; import org.json.JSONObject; +import java.io.Closeable; import java.lang.ref.WeakReference; import java.util.Observable; import java.util.Observer; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.atomic.AtomicInteger; import de.blinkt.openvpn.LaunchVPN; @@ -51,6 +54,7 @@ import se.leap.bitmaskclient.OnBootReceiver; import static android.app.Activity.RESULT_CANCELED; import static android.app.Activity.RESULT_OK; import static android.content.Intent.CATEGORY_DEFAULT; +import static se.leap.bitmaskclient.ConfigHelper.ensureNotOnMainThread; import static se.leap.bitmaskclient.Constants.BROADCAST_EIP_EVENT; import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_CODE; import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_KEY; @@ -91,10 +95,7 @@ public final class EIP extends Service implements Observer { private AtomicInteger processCounter; private EipStatus eipStatus; - private AtomicBoolean disconnectOnConnect = new AtomicBoolean(false); - - private IOpenVPNServiceInternal mService; - private ServiceConnection openVpnConnection; + private OpenvpnServiceConnection openvpnServiceConnection; @Nullable @Override @@ -115,8 +116,9 @@ public final class EIP extends Service implements Observer { public void onDestroy() { super.onDestroy(); eipStatus.deleteObserver(this); - if (mService != null) { - unbindService(openVpnConnection); + if (openvpnServiceConnection != null) { + openvpnServiceConnection.close(); + openvpnServiceConnection = null; } } @@ -226,13 +228,10 @@ public final class EIP extends Service implements Observer { * First checks if the OpenVpnConnection is open then * terminates EIP if currently connected or connecting */ + @WorkerThread private void stopEIP() { // if the OpenVpnConnection doesn't exist remember to disconnect after connecting - int resultCode = RESULT_CANCELED; - if (eipStatus.isConnected() || eipStatus.isConnecting()) { - stop(); - resultCode = RESULT_OK; - } + int resultCode = stop() ? RESULT_OK : RESULT_CANCELED; tellToReceiverOrBroadcast(EIP_ACTION_STOP, resultCode); } @@ -343,12 +342,13 @@ public final class EIP extends Service implements Observer { * disable Bitmask starting on after phone reboot * then stop VPN */ - private void stop() { + @WorkerThread + private boolean stop() { preferences.edit().putBoolean(EIP_RESTART_ON_BOOT, false).apply(); if (eipStatus.isBlockingVpnEstablished()) { stopBlockingVpn(); } - disconnect(); + return disconnect(); } /** @@ -366,46 +366,79 @@ public final class EIP extends Service implements Observer { * check if OpenVPNConnection is connected * then terminate VPN connection */ - private void disconnect() { + @WorkerThread + private boolean disconnect() { // if OpenVPNConnection is not connected remember to disconnect // after connection is established - if (mService == null) { - disconnectOnConnect.set(true); - bindOpenVpnService(); - return; + if (openvpnServiceConnection == null) { + try { + openvpnServiceConnection = new OpenvpnServiceConnection(this); + } catch (InterruptedException | IllegalStateException e) { + e.printStackTrace(); + return false; + } } ProfileManager.setConntectedVpnProfileDisconnected(this); - if (mService != null) { - try { - mService.stopVPN(false); - } catch (RemoteException e) { - VpnStatus.logException(e); - } + try { + return openvpnServiceConnection.getService().stopVPN(false); + } catch (RemoteException e) { + VpnStatus.logException(e); } + return false; } /** - * bind OpenVPNService to openVPNConnection + * Creates a service connection to OpenVpnService. + * The constructor blocks until the service is bound to the given Context. + * Pattern stolen from android.security.KeyChain.java */ - private void bindOpenVpnService() { - openVpnConnection = new ServiceConnection() { - @Override - public void onServiceConnected(ComponentName className, IBinder service) { - mService = IOpenVPNServiceInternal.Stub.asInterface(service); - if (disconnectOnConnect.get()) { - disconnect(); - disconnectOnConnect.set(false); + @WorkerThread + public static class OpenvpnServiceConnection implements Closeable { + private final Context context; + private ServiceConnection serviceConnection; + private IOpenVPNServiceInternal service; + + + + protected OpenvpnServiceConnection(Context context) throws InterruptedException { + this.context = context; + ensureNotOnMainThread(context); + Log.d(TAG, "initSynchronzedServiceConnection!"); + initSynchronizedServiceConnection(context); + } + + private void initSynchronizedServiceConnection(Context context) throws InterruptedException { + final BlockingQueue blockingQueue = new LinkedBlockingQueue<>(1); + this.serviceConnection = new ServiceConnection() { + volatile boolean mConnectedAtLeastOnce = false; + @Override public void onServiceConnected(ComponentName name, IBinder service) { + if (!mConnectedAtLeastOnce) { + mConnectedAtLeastOnce = true; + try { + blockingQueue.put(IOpenVPNServiceInternal.Stub.asInterface(service)); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } } - } - @Override - public void onServiceDisconnected(ComponentName arg0) { - mService = null; - } - }; - Intent intent = new Intent(this, OpenVPNService.class); - intent.setAction(OpenVPNService.START_SERVICE); - bindService(intent, openVpnConnection, Context.BIND_AUTO_CREATE); + @Override public void onServiceDisconnected(ComponentName name) { + } + }; + + Intent intent = new Intent(context, OpenVPNService.class); + intent.setAction(OpenVPNService.START_SERVICE); + context.bindService(intent, serviceConnection, Context.BIND_AUTO_CREATE); + service = blockingQueue.take(); + } + + @Override public void close() { + context.unbindService(serviceConnection); + } + + public IOpenVPNServiceInternal getService() { + return service; + } } /** -- cgit v1.2.3