From 8364aa15f1d03352d8db3de85fd383943e2f457e Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:11:00 +1100 Subject: [PATCH 1/9] Don't try to start swap again if we have gone from network -> no network. --- .../org/fdroid/fdroid/localrepo/SwapService.java | 15 ++++++++++----- .../fdroid/fdroid/net/WifiStateChangeService.java | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java b/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java index 0dc8380c3..d82934152 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java @@ -441,14 +441,18 @@ public class SwapService extends Service { /** * Handles checking if the {@link SwapService} is running, and only restarts it if it was running. */ - public void restartWifiIfEnabled() { + public void stopWifiIfEnabled(final boolean restartAfterStopping) { if (wifiSwap.isConnected()) { new AsyncTask() { @Override protected Void doInBackground(Void... params) { - Utils.debugLog(TAG, "Restarting WiFi swap service"); + Utils.debugLog(TAG, "Stopping the currently running WiFi swap service (on background thread)"); wifiSwap.stop(); - wifiSwap.start(); + + if (restartAfterStopping) { + Utils.debugLog(TAG, "Restarting WiFi swap service after stopping (still on background thread)"); + wifiSwap.start(); + } return null; } }.execute(); @@ -632,7 +636,7 @@ public class SwapService extends Service { @Override public void onPreferenceChange() { Log.i(TAG, "Swap over HTTPS preference changed."); - restartWifiIfEnabled(); + stopWifiIfEnabled(true); } }; @@ -640,7 +644,8 @@ public class SwapService extends Service { private final BroadcastReceiver onWifiChange = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent i) { - restartWifiIfEnabled(); + boolean hasIp = FDroidApp.ipAddressString != null; + stopWifiIfEnabled(hasIp); } }; diff --git a/F-Droid/src/org/fdroid/fdroid/net/WifiStateChangeService.java b/F-Droid/src/org/fdroid/fdroid/net/WifiStateChangeService.java index 0e18b6af7..ccf162460 100644 --- a/F-Droid/src/org/fdroid/fdroid/net/WifiStateChangeService.java +++ b/F-Droid/src/org/fdroid/fdroid/net/WifiStateChangeService.java @@ -181,7 +181,7 @@ public class WifiStateChangeService extends Service { getApplicationContext().bindService(swapService, new ServiceConnection() { @Override public void onServiceConnected(ComponentName name, IBinder service) { - ((SwapService.Binder) service).getService().restartWifiIfEnabled(); + ((SwapService.Binder) service).getService().stopWifiIfEnabled(true); getApplicationContext().unbindService(this); } From 1eae13592909894bc04db0cb4d88bd345215efa6 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:13:33 +1100 Subject: [PATCH 2/9] Add user feedback for when WiFi is being disabled. On some devices this can take some time (i.e. a second) and the UI needs to be disabled for that time. This should stop users quickly stopping and starting regularly, queuing up many "start jmdns, stop jmdns, start jmdns" calls. --- F-Droid/res/values/strings.xml | 1 + F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java | 1 + F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java | 1 + .../src/org/fdroid/fdroid/views/swap/StartSwapView.java | 7 ++++++- 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/F-Droid/res/values/strings.xml b/F-Droid/res/values/strings.xml index 861112c1c..bd55734cb 100644 --- a/F-Droid/res/values/strings.xml +++ b/F-Droid/res/values/strings.xml @@ -322,6 +322,7 @@ Not visible via Bluetooth Visible via Wi-Fi Setting up Wi-Fi… + Stopping Wi-Fi… Not visible via Wi-Fi Device Name Can\'t find who you\'re looking for? diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java b/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java index d82934152..7203aefa5 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/SwapService.java @@ -487,6 +487,7 @@ public class SwapService extends Service { public static final String WIFI_STATE_CHANGE = "org.fdroid.fdroid.WIFI_STATE_CHANGE"; public static final String EXTRA_STARTING = "STARTING"; public static final String EXTRA_STARTED = "STARTED"; + public static final String EXTRA_STOPPING = "STOPPING"; public static final String EXTRA_STOPPED = "STOPPED"; private static final int NOTIFICATION = 1; diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java index d66e5ed33..70634d28e 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java @@ -91,6 +91,7 @@ public class WifiSwap extends SwapType { @Override public void stop() { + sendBroadcast(SwapService.EXTRA_STOPPING); if (webServerThreadHandler == null) { Log.i(TAG, "null handler in stopWebServer"); } else { diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java b/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java index bbc17b184..f653e1944 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java @@ -323,10 +323,15 @@ public class StartSwapView extends ScrollView implements SwapWorkflowActivity.In @Override public void onReceive(Context context, Intent intent) { if (intent.hasExtra(SwapService.EXTRA_STARTING)) { - Utils.debugLog(TAG, "WiFi service is starting (setting toggle to visible, but disabled)."); + Utils.debugLog(TAG, "WiFi service is starting (setting toggle to checked, but disabled)."); textWifiVisible.setText(R.string.swap_setting_up_wifi); wifiSwitch.setEnabled(false); wifiSwitch.setChecked(true); + } else if (intent.hasExtra(SwapService.EXTRA_STOPPING)) { + Utils.debugLog(TAG, "WiFi service is stopping (setting toggle to unchecked and disabled)."); + textWifiVisible.setText(R.string.swap_stopping_wifi); + wifiSwitch.setEnabled(false); + wifiSwitch.setChecked(false); } else { wifiSwitch.setEnabled(true); if (intent.hasExtra(SwapService.EXTRA_STARTED)) { From ddbd9e2ea947d192e816955619c8270783b9b27d Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:15:01 +1100 Subject: [PATCH 3/9] Actually run thread in background. `Thread.run()` is not the correct call, changed to the correct `Thread.start()`. Also, explicitly indicate that we want the stopping of wifi to happen in the background. --- F-Droid/src/org/fdroid/fdroid/localrepo/type/SwapType.java | 2 +- F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/SwapType.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/SwapType.java index 304be4cbb..1a2ee1a78 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/SwapType.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/SwapType.java @@ -99,7 +99,7 @@ public abstract class SwapType { public void run() { SwapType.this.stop(); } - }.run(); + }.start(); } } diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java b/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java index f653e1944..2923865cf 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/StartSwapView.java @@ -302,9 +302,8 @@ public class StartSwapView extends ScrollView implements SwapWorkflowActivity.In Utils.debugLog(TAG, "Received onCheckChanged(true) for WiFi swap, asking in background thread to ensure WiFi swap is running."); getManager().getWifiSwap().ensureRunningInBackground(); } else { - Utils.debugLog(TAG, "Received onCheckChanged(false) for WiFi swap, disabling WiFi swap."); - getManager().getWifiSwap().stop(); - Utils.debugLog(TAG, "Received onCheckChanged(false) for WiFi swap, WiFi swap disabled successfully."); + Utils.debugLog(TAG, "Received onCheckChanged(false) for WiFi swap, disabling WiFi swap in background thread."); + getManager().getWifiSwap().stopInBackground(); } uiUpdateWifiNetwork(); } From 18f97602e8b9ae8f791b5b135732265dcc4bab3a Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:16:00 +1100 Subject: [PATCH 4/9] Don't start swap wifi when there is no network. Not sure if we should be here or not in this situation, so this is a little bit defensive. Can't bind to an IP address of `null`, so don't bother starting LocalHTTP server unless we have an IP. --- F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java index 70634d28e..0ce4bcc4f 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java @@ -42,10 +42,14 @@ public class WifiSwap extends SwapType { @Override public void start() { - Utils.debugLog(TAG, "Preparing swap webserver."); sendBroadcast(SwapService.EXTRA_STARTING); + if (FDroidApp.ipAddressString == null) { + Log.e(TAG, "Not starting swap webserver, because we don't seem to be connected to a network."); + setConnected(false); + } + Runnable webServer = new Runnable() { // Tell Eclipse this is not a leak because of Looper use. @SuppressLint("HandlerLeak") From 2ed6110ae541a9929c1748c8aaf6fc32f52047d4 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:16:48 +1100 Subject: [PATCH 5/9] Cleanup looper after shutting down. --- F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java index 0ce4bcc4f..d1e070e8c 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java @@ -69,6 +69,12 @@ public class WifiSwap extends SwapType { Log.i(TAG, "we've been asked to stop the webserver: " + msg.obj); setConnected(false); localHttpd.stop(); + Looper looper = Looper.myLooper(); + if (looper == null) { + Log.e(TAG, "Looper.myLooper() was null for sum reason while shutting down the swap webserver."); + } else { + looper.quit(); + } } }; try { From 1505d21781b2306e17f5f592d7bda459b819289e Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:19:29 +1100 Subject: [PATCH 6/9] Set connected status after disconnecting from wifi swap. --- F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java index d1e070e8c..ac8a8c4a2 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java @@ -110,7 +110,13 @@ public class WifiSwap extends SwapType { msg.obj = webServerThreadHandler.getLooper().getThread().getName() + " says stop"; webServerThreadHandler.sendMessage(msg); } + + // Stop the Bonjour stuff after asking the webserver to stop. This is not required in this + // order, but it helps. In practice, the Bonjour stuff takes a second or two to stop. This + // should give enough time for the message we posted above to reach the web server thread + // and for the webserver to thus be stopped. bonjourBroadcast.stop(); + setConnected(false); } } From cd3a064a5a5c37967fd0591edaefcb29e221a187 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:31:38 +1100 Subject: [PATCH 7/9] Don't call `unregisterService(...)` then `unregisterAllSerivces()`. The later will unregister the service we were explicitly unregistering anyway. --- .../org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java index a5ab3a6b9..7497ca68b 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java @@ -78,12 +78,9 @@ public class BonjourBroadcast extends SwapType { private void clearCurrentMDNSService() { if (jmdns != null) { - if (pairService != null) { - jmdns.unregisterService(pairService); - pairService = null; - } jmdns.unregisterAllServices(); Utils.closeQuietly(jmdns); + pairService = null; jmdns = null; } } From 3dd0589b0833cd70e46b0c1a95f4695419b2e73c Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:32:40 +1100 Subject: [PATCH 8/9] Don't say wifi is connected until both wifi + bonjour are ready. Although I can't reproduce reliably, I am hopeful that this will resolve a lot of the errors such as #557. --- .../fdroid/localrepo/type/WifiSwap.java | 142 ++++++++++++------ 1 file changed, 99 insertions(+), 43 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java index ac8a8c4a2..8229f692f 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/WifiSwap.java @@ -19,6 +19,13 @@ import java.io.IOException; import java.net.BindException; import java.util.Random; +import rx.Single; +import rx.SingleSubscriber; +import rx.android.schedulers.AndroidSchedulers; +import rx.functions.Action1; +import rx.functions.Func2; +import rx.schedulers.Schedulers; + public class WifiSwap extends SwapType { private static final String TAG = "WifiSwap"; @@ -49,54 +56,103 @@ public class WifiSwap extends SwapType { Log.e(TAG, "Not starting swap webserver, because we don't seem to be connected to a network."); setConnected(false); } - - Runnable webServer = new Runnable() { - // Tell Eclipse this is not a leak because of Looper use. - @SuppressLint("HandlerLeak") - @Override - public void run() { - localHttpd = new LocalHTTPD( - context, - FDroidApp.ipAddressString, - FDroidApp.port, - context.getFilesDir(), - Preferences.get().isLocalRepoHttpsEnabled()); - Looper.prepare(); // must be run before creating a Handler - webServerThreadHandler = new Handler() { + Single.zip( + Single.create(getWebServerTask()), + Single.create(getBonjourTask()), + new Func2() { @Override - public void handleMessage(Message msg) { - Log.i(TAG, "we've been asked to stop the webserver: " + msg.obj); - setConnected(false); - localHttpd.stop(); - Looper looper = Looper.myLooper(); - if (looper == null) { - Log.e(TAG, "Looper.myLooper() was null for sum reason while shutting down the swap webserver."); - } else { - looper.quit(); - } + public Boolean call(Boolean webServerTask, Boolean bonjourServiceTask) { + return bonjourServiceTask && webServerTask; } - }; - try { - Utils.debugLog(TAG, "Starting swap webserver..."); - localHttpd.start(); - setConnected(true); - Utils.debugLog(TAG, "Swap webserver started."); - } catch (BindException e) { - int prev = FDroidApp.port; - FDroidApp.port = FDroidApp.port + new Random().nextInt(1111); - setConnected(false); - Log.w(TAG, "port " + prev + " occupied, trying on " + FDroidApp.port + "!"); - context.startService(new Intent(context, WifiStateChangeService.class)); - } catch (IOException e) { - setConnected(false); - Log.e(TAG, "Could not start local repo HTTP server", e); - } - Looper.loop(); // start the message receiving loop + }) + .observeOn(AndroidSchedulers.mainThread()) + .subscribeOn(Schedulers.newThread()) + .subscribe(new Action1() { + @Override + public void call(Boolean success) { + setConnected(success); + } + }, + new Action1() { + @Override + public void call(Throwable throwable) { + setConnected(false); + } + }); + } + + /** + * A task which starts the {@link WifiSwap#bonjourBroadcast} and then emits a `true` value at + * the end. + */ + private Single.OnSubscribe getBonjourTask() { + return new Single.OnSubscribe() { + @Override + public void call(SingleSubscriber singleSubscriber) { + bonjourBroadcast.start(); + + // TODO: Be more intelligent about failures here so that we can invoke + // singleSubscriber.onError() in the appropriate circumstances. + singleSubscriber.onSuccess(true); + } + }; + } + + /** + * Constructs a new {@link Thread} for the webserver to run on. If successful, it will also + * populate the webServerThreadHandler property and bind it to that particular thread. This + * allows messages to be sent to the webserver thread by posting messages to that handler. + */ + private Single.OnSubscribe getWebServerTask() { + return new Single.OnSubscribe() { + @Override + public void call(final SingleSubscriber singleSubscriber) { + new Thread(new Runnable() { + // Tell Eclipse this is not a leak because of Looper use. + @SuppressLint("HandlerLeak") + @Override + public void run() { + localHttpd = new LocalHTTPD( + context, + FDroidApp.ipAddressString, + FDroidApp.port, + context.getFilesDir(), + Preferences.get().isLocalRepoHttpsEnabled()); + + Looper.prepare(); // must be run before creating a Handler + webServerThreadHandler = new Handler() { + @Override + public void handleMessage(Message msg) { + Log.i(TAG, "we've been asked to stop the webserver: " + msg.obj); + localHttpd.stop(); + Looper looper = Looper.myLooper(); + if (looper == null) { + Log.e(TAG, "Looper.myLooper() was null for sum reason while shutting down the swap webserver."); + } else { + looper.quit(); + } + } + }; + try { + Utils.debugLog(TAG, "Starting swap webserver..."); + localHttpd.start(); + Utils.debugLog(TAG, "Swap webserver started."); + singleSubscriber.onSuccess(true); + } catch (BindException e) { + int prev = FDroidApp.port; + FDroidApp.port = FDroidApp.port + new Random().nextInt(1111); + context.startService(new Intent(context, WifiStateChangeService.class)); + singleSubscriber.onError(new Exception("port " + prev + " occupied, trying on " + FDroidApp.port + "!")); + } catch (IOException e) { + Log.e(TAG, "Could not start local repo HTTP server", e); + singleSubscriber.onError(e); + } + Looper.loop(); // start the message receiving loop + } + }).start(); } }; - new Thread(webServer).start(); - bonjourBroadcast.start(); } @Override From db120133b9edf03f4226126a2820805bbb909573 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 26 Feb 2016 09:31:08 +1100 Subject: [PATCH 9/9] Don't try to start bonjour without an IP. Although not reproduced, it looks very much like this would be related to, and should subsequently fix #556. --- .../localrepo/type/BonjourBroadcast.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java b/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java index 7497ca68b..11711ee49 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java @@ -1,6 +1,7 @@ package org.fdroid.fdroid.localrepo.type; import android.content.Context; +import android.support.annotation.Nullable; import android.util.Log; import org.fdroid.fdroid.FDroidApp; @@ -10,6 +11,7 @@ import org.fdroid.fdroid.localrepo.SwapService; import java.io.IOException; import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.HashMap; import javax.jmdns.JmDNS; @@ -31,10 +33,15 @@ public class BonjourBroadcast extends SwapType { @Override public void start() { - Utils.debugLog(TAG, "Preparing to start Bonjour service."); sendBroadcast(SwapService.EXTRA_STARTING); + InetAddress address = getDeviceAddress(); + if (address == null) { + Log.e(TAG, "Starting Bonjour service, but couldn't ascertain IP address. Seems we are not connected to a network."); + return; + } + /* * a ServiceInfo can only be registered with a single instance * of JmDNS, and there is only ever a single LocalHTTPD port to @@ -43,6 +50,7 @@ public class BonjourBroadcast extends SwapType { if (pairService != null || jmdns != null) { clearCurrentMDNSService(); } + String repoName = Preferences.get().getLocalRepoName(); HashMap values = new HashMap<>(); values.put("path", "/fdroid/repo"); @@ -59,7 +67,7 @@ public class BonjourBroadcast extends SwapType { try { Utils.debugLog(TAG, "Starting bonjour service..."); pairService = ServiceInfo.create(type, repoName, FDroidApp.port, 0, 0, values); - jmdns = JmDNS.create(InetAddress.getByName(FDroidApp.ipAddressString)); + jmdns = JmDNS.create(address); jmdns.registerService(pairService); setConnected(true); Utils.debugLog(TAG, "... Bounjour service started."); @@ -90,4 +98,15 @@ public class BonjourBroadcast extends SwapType { return SwapService.BONJOUR_STATE_CHANGE; } + @Nullable + private InetAddress getDeviceAddress() { + if (FDroidApp.ipAddressString != null) { + try { + return InetAddress.getByName(FDroidApp.ipAddressString); + } catch (UnknownHostException e) { } + } + + return null; + } + }