From 7eeab77aaf7938609a8babb38b6d8b1a3f1cbfd2 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 12 May 2016 22:35:18 +0200 Subject: [PATCH 1/4] in ACRA process, do not run everything in FDroidApp.onCreate() The `android:process` statement in AndroidManifest.xml causes another process to be created to run CrashReportActivity. This was causing lots of things to be started/run twice including CleanCacheService and WifiStateChangeService. --- app/src/main/java/org/fdroid/fdroid/FDroidApp.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index c774fccd9..7062d3c3f 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -20,6 +20,8 @@ package org.fdroid.fdroid; import android.annotation.TargetApi; import android.app.Activity; +import android.app.ActivityManager; +import android.app.ActivityManager.RunningAppProcessInfo; import android.app.Application; import android.bluetooth.BluetoothAdapter; import android.bluetooth.BluetoothManager; @@ -183,7 +185,16 @@ public class FDroidApp extends Application { .build()); } updateLanguage(); + ACRA.init(this); + // if this is the ACRA process, do not run the rest of onCreate() + int pid = android.os.Process.myPid(); + ActivityManager manager = (ActivityManager) this.getSystemService(Context.ACTIVITY_SERVICE); + for (RunningAppProcessInfo processInfo : manager.getRunningAppProcesses()) { + if (processInfo.pid == pid && "org.fdroid.fdroid:acra".equals(processInfo.processName)) { + return; + } + } PRNGFixes.apply(); From 26d173acdc776a4181d1ed1b54d752d43518b815 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 12 May 2016 22:45:28 +0200 Subject: [PATCH 2/4] convert WifiStateChangeService to IntentService The IntentService provides the nice incoming Intent queue. It also runs the Intent in a thread, so even the initial check is now in a very low priority thread. The queuing prevents the incoming Intents from competing. This also simplifies the code since the lifecycle is more automatic now. --- .../fdroid/net/WifiStateChangeService.java | 96 ++++++++----------- 1 file changed, 39 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java b/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java index 160ca6197..a402246d8 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java @@ -1,18 +1,14 @@ package org.fdroid.fdroid.net; import android.annotation.TargetApi; -import android.app.Service; -import android.content.ComponentName; +import android.app.IntentService; import android.content.Context; import android.content.Intent; -import android.content.ServiceConnection; import android.net.DhcpInfo; import android.net.NetworkInfo; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; -import android.os.AsyncTask; import android.os.Build; -import android.os.IBinder; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; @@ -23,7 +19,6 @@ import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.localrepo.LocalRepoKeyStore; import org.fdroid.fdroid.localrepo.LocalRepoManager; -import org.fdroid.fdroid.localrepo.SwapService; import java.net.Inet6Address; import java.net.InetAddress; @@ -34,54 +29,63 @@ import java.security.cert.Certificate; import java.util.Enumeration; import java.util.Locale; -public class WifiStateChangeService extends Service { +/** + * Handle state changes to the device's wifi, storing the required bits. + * The {@link Intent} that starts it either has no extras included, + * which is how it can be triggered by code, or it came in from the system + * via {@link org.fdroid.fdroid.receiver.WifiStateChangeReceiver}, in + * which case an instance of {@link NetworkInfo} is included. + */ +public class WifiStateChangeService extends IntentService { private static final String TAG = "WifiStateChangeService"; public static final String BROADCAST = "org.fdroid.fdroid.action.WIFI_CHANGE"; private WifiManager wifiManager; - private static WaitForWifiAsyncTask asyncTask; - private int wifiState; + private static WifiInfoThread wifiInfoThread; + + public WifiStateChangeService() { + super("WifiStateChangeService"); + } @Override - public int onStartCommand(Intent intent, int flags, int startId) { + protected void onHandleIntent(Intent intent) { + android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_LOWEST); Utils.debugLog(TAG, "WiFi change service started, clearing info about wifi state until we have figured it out again."); FDroidApp.initWifiSettings(); NetworkInfo ni = intent.getParcelableExtra(WifiManager.EXTRA_NETWORK_INFO); wifiManager = (WifiManager) getSystemService(WIFI_SERVICE); - wifiState = wifiManager.getWifiState(); + int wifiState = wifiManager.getWifiState(); if (ni == null || ni.isConnected()) { - /* started on app start or from WifiStateChangeReceiver, - NetworkInfo is only passed via WifiStateChangeReceiver */ Utils.debugLog(TAG, "ni == " + ni + " wifiState == " + printWifiState(wifiState)); if (wifiState == WifiManager.WIFI_STATE_ENABLED || wifiState == WifiManager.WIFI_STATE_DISABLING // might be switching to hotspot || wifiState == WifiManager.WIFI_STATE_DISABLED // might be hotspot || wifiState == WifiManager.WIFI_STATE_UNKNOWN) { // might be hotspot - if (asyncTask != null) { - asyncTask.cancel(true); + if (wifiInfoThread != null) { + wifiInfoThread.interrupt(); } - asyncTask = new WaitForWifiAsyncTask(); - asyncTask.execute(); + wifiInfoThread = new WifiInfoThread(); + wifiInfoThread.start(); } } - return START_NOT_STICKY; } - public class WaitForWifiAsyncTask extends AsyncTask { - private static final String TAG = "WaitForWifiAsyncTask"; + public class WifiInfoThread extends Thread { + private static final String TAG = "WifiInfoThread"; @Override - protected Void doInBackground(Void... params) { + public void run() { + android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_LOWEST); try { Utils.debugLog(TAG, "Checking wifi state (in background thread)."); WifiInfo wifiInfo = null; - wifiState = wifiManager.getWifiState(); + int wifiState = wifiManager.getWifiState(); while (FDroidApp.ipAddressString == null) { - if (isCancelled()) { // can be canceled by a change via WifiStateChangeReceiver - return null; + if (isInterrupted()) { // can be canceled by a change via WifiStateChangeReceiver + return; } if (wifiState == WifiManager.WIFI_STATE_ENABLED) { wifiInfo = wifiManager.getConnectionInfo(); @@ -98,7 +102,7 @@ public class WifiStateChangeService extends Service { // try once to see if its a hotspot setIpInfoFromNetworkInterface(); if (FDroidApp.ipAddressString == null) { - return null; + return; } } else { // a hotspot can be active during WIFI_STATE_UNKNOWN setIpInfoFromNetworkInterface(); @@ -109,8 +113,8 @@ public class WifiStateChangeService extends Service { Utils.debugLog(TAG, "waiting for an IP address..."); } } - if (isCancelled()) { // can be canceled by a change via WifiStateChangeReceiver - return null; + if (isInterrupted()) { // can be canceled by a change via WifiStateChangeReceiver + return; } if (wifiInfo != null) { @@ -125,7 +129,6 @@ public class WifiStateChangeService extends Service { } } - // TODO: Can this be moved to the swap service instead? String scheme; if (Preferences.get().isLocalRepoHttpsEnabled()) { scheme = "https"; @@ -136,16 +139,16 @@ public class WifiStateChangeService extends Service { FDroidApp.REPO.address = String.format(Locale.ENGLISH, "%s://%s:%d/fdroid/repo", scheme, FDroidApp.ipAddressString, FDroidApp.port); - if (isCancelled()) { // can be canceled by a change via WifiStateChangeReceiver - return null; + if (isInterrupted()) { // can be canceled by a change via WifiStateChangeReceiver + return; } Context context = WifiStateChangeService.this.getApplicationContext(); LocalRepoManager lrm = LocalRepoManager.get(context); lrm.writeIndexPage(Utils.getSharingUri(FDroidApp.REPO).toString()); - if (isCancelled()) { // can be canceled by a change via WifiStateChangeReceiver - return null; + if (isInterrupted()) { // can be canceled by a change via WifiStateChangeReceiver + return; } // the fingerprint for the local repo's signing key @@ -164,38 +167,17 @@ public class WifiStateChangeService extends Service { localRepoKeyStore.setupHTTPSCertificate(); } - } catch (LocalRepoKeyStore.InitException | InterruptedException e) { + } catch (LocalRepoKeyStore.InitException e) { Log.e(TAG, "Unable to configure a fingerprint or HTTPS for the local repo", e); + } catch (InterruptedException e) { + Utils.debugLog(TAG, "interrupted"); + return; } - return null; - } - - @Override - protected void onPostExecute(Void result) { Intent intent = new Intent(BROADCAST); LocalBroadcastManager.getInstance(WifiStateChangeService.this).sendBroadcast(intent); - WifiStateChangeService.this.stopSelf(); - - Intent swapService = new Intent(WifiStateChangeService.this, SwapService.class); - getApplicationContext().bindService(swapService, new ServiceConnection() { - @Override - public void onServiceConnected(ComponentName name, IBinder service) { - ((SwapService.Binder) service).getService().stopWifiIfEnabled(true); - getApplicationContext().unbindService(this); - } - - @Override - public void onServiceDisconnected(ComponentName name) { - } - }, BIND_AUTO_CREATE); } } - @Override - public IBinder onBind(Intent intent) { - return null; - } - @TargetApi(9) private void setIpInfoFromNetworkInterface() { try { From 426e03a6498660b54ddf18f7c4f20e366394d7f3 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 13 May 2016 12:49:08 +0200 Subject: [PATCH 3/4] switch out swap Repo instance all at once Before, it would change fields in a final Repo instance, which means that things could be out of sync when accessed. Now it swaps out the old one with a new Repo instance in one step. The local repo variables are now declared volatile so that they are more predictable when accessed from various threads (WifiStateChangeService, SwapService, etc.) askServerToSwapWithUs(NewRepoConfig) was unused, so I removed it. --- app/src/main/java/org/fdroid/fdroid/FDroidApp.java | 13 +++++++------ .../org/fdroid/fdroid/localrepo/SwapService.java | 7 +------ .../fdroid/localrepo/peers/BonjourFinder.java | 2 +- .../fdroid/localrepo/type/BonjourBroadcast.java | 2 +- .../fdroid/fdroid/net/WifiStateChangeService.java | 12 ++++++++---- .../fdroid/views/swap/SwapWorkflowActivity.java | 4 ++-- .../org/fdroid/fdroid/views/swap/WifiQrView.java | 4 ++-- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 7062d3c3f..882654abd 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -81,12 +81,12 @@ public class FDroidApp extends Application { private static Locale locale; // for the local repo on this device, all static since there is only one - public static int port; - public static String ipAddressString; - public static SubnetUtils.SubnetInfo subnetInfo; - public static String ssid; - public static String bssid; - public static final Repo REPO = new Repo(); + public static volatile int port; + public static volatile String ipAddressString; + public static volatile SubnetUtils.SubnetInfo subnetInfo; + public static volatile String ssid; + public static volatile String bssid; + public static volatile Repo repo = new Repo(); // Leaving the fully qualified class name here to help clarify the difference between spongy/bouncy castle. private static final org.spongycastle.jce.provider.BouncyCastleProvider SPONGYCASTLE_PROVIDER; @@ -147,6 +147,7 @@ public class FDroidApp extends Application { subnetInfo = new SubnetUtils("0.0.0.0/32").getInfo(); ssid = ""; bssid = ""; + repo = new Repo(); } public void updateLanguage() { diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java index a9aedff12..fcadd5176 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java @@ -31,7 +31,6 @@ import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; import org.fdroid.fdroid.UpdateService; import org.fdroid.fdroid.Utils; -import org.fdroid.fdroid.data.NewRepoConfig; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.localrepo.peers.Peer; @@ -208,16 +207,12 @@ public class SwapService extends Service { askServerToSwapWithUs(repo.address); } - public void askServerToSwapWithUs(final NewRepoConfig config) { - askServerToSwapWithUs(config.getRepoUriString()); - } - private void askServerToSwapWithUs(final String address) { new AsyncTask() { @Override protected Void doInBackground(Void... args) { Uri repoUri = Uri.parse(address); - String swapBackUri = Utils.getLocalRepoUri(FDroidApp.REPO).toString(); + String swapBackUri = Utils.getLocalRepoUri(FDroidApp.repo).toString(); AndroidHttpClient client = AndroidHttpClient.newInstance("F-Droid", SwapService.this); HttpPost request = new HttpPost("/request-swap"); diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/peers/BonjourFinder.java b/app/src/main/java/org/fdroid/fdroid/localrepo/peers/BonjourFinder.java index 4a83dc512..5b506c3f5 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/peers/BonjourFinder.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/peers/BonjourFinder.java @@ -130,7 +130,7 @@ class BonjourFinder extends PeerFinder implements ServiceListener { final String type = serviceInfo.getPropertyString("type"); final String fingerprint = serviceInfo.getPropertyString("fingerprint"); final boolean isFDroid = type != null && type.startsWith("fdroidrepo"); - final boolean isSelf = FDroidApp.REPO != null && fingerprint != null && fingerprint.equalsIgnoreCase(FDroidApp.REPO.fingerprint); + final boolean isSelf = FDroidApp.repo != null && fingerprint != null && fingerprint.equalsIgnoreCase(FDroidApp.repo.fingerprint); if (isFDroid && !isSelf) { Utils.debugLog(TAG, "Found F-Droid swap Bonjour service:\n" + serviceInfo); subscriber.onNext(new BonjourPeer(serviceInfo)); diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java b/app/src/main/java/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java index d5e28c1c8..9d5b56af4 100644 --- a/app/src/main/java/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java +++ b/app/src/main/java/org/fdroid/fdroid/localrepo/type/BonjourBroadcast.java @@ -55,7 +55,7 @@ public class BonjourBroadcast extends SwapType { HashMap values = new HashMap<>(); values.put("path", "/fdroid/repo"); values.put("name", repoName); - values.put("fingerprint", FDroidApp.REPO.fingerprint); + values.put("fingerprint", FDroidApp.repo.fingerprint); String type; if (Preferences.get().isLocalRepoHttpsEnabled()) { values.put("type", "fdroidrepos"); diff --git a/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java b/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java index a402246d8..dda912c58 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java @@ -17,6 +17,7 @@ import org.apache.commons.net.util.SubnetUtils; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.localrepo.LocalRepoKeyStore; import org.fdroid.fdroid.localrepo.LocalRepoManager; @@ -135,8 +136,9 @@ public class WifiStateChangeService extends IntentService { } else { scheme = "http"; } - FDroidApp.REPO.name = Preferences.get().getLocalRepoName(); - FDroidApp.REPO.address = String.format(Locale.ENGLISH, "%s://%s:%d/fdroid/repo", + Repo repo = new Repo(); + repo.name = Preferences.get().getLocalRepoName(); + repo.address = String.format(Locale.ENGLISH, "%s://%s:%d/fdroid/repo", scheme, FDroidApp.ipAddressString, FDroidApp.port); if (isInterrupted()) { // can be canceled by a change via WifiStateChangeReceiver @@ -145,7 +147,7 @@ public class WifiStateChangeService extends IntentService { Context context = WifiStateChangeService.this.getApplicationContext(); LocalRepoManager lrm = LocalRepoManager.get(context); - lrm.writeIndexPage(Utils.getSharingUri(FDroidApp.REPO).toString()); + lrm.writeIndexPage(Utils.getSharingUri(FDroidApp.repo).toString()); if (isInterrupted()) { // can be canceled by a change via WifiStateChangeReceiver return; @@ -154,7 +156,9 @@ public class WifiStateChangeService extends IntentService { // the fingerprint for the local repo's signing key LocalRepoKeyStore localRepoKeyStore = LocalRepoKeyStore.get(context); Certificate localCert = localRepoKeyStore.getCertificate(); - FDroidApp.REPO.fingerprint = Utils.calcFingerprint(localCert); + repo.fingerprint = Utils.calcFingerprint(localCert); + + FDroidApp.repo = repo; /* * Once the IP address is known we need to generate a self diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java index c711d3ec6..6062d643e 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java @@ -504,7 +504,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { // Even if they opted to skip the message which says "Touch devices to swap", // we still want to actually enable the feature, so that they could touch // during the wifi qr code being shown too. - boolean nfcMessageReady = NfcHelper.setPushMessage(this, Utils.getSharingUri(FDroidApp.REPO)); + boolean nfcMessageReady = NfcHelper.setPushMessage(this, Utils.getSharingUri(FDroidApp.repo)); if (Preferences.get().showNfcDuringSwap() && nfcMessageReady) { inflateInnerView(R.layout.swap_nfc); @@ -669,7 +669,7 @@ public class SwapWorkflowActivity extends AppCompatActivity { PrepareSwapRepo(@NonNull Set apps) { context = SwapWorkflowActivity.this; selectedApps = apps; - sharingUri = Utils.getSharingUri(FDroidApp.REPO); + sharingUri = Utils.getSharingUri(FDroidApp.repo); } private void broadcast(int type) { diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/WifiQrView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/WifiQrView.java index b4f25e050..c77c06e22 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/WifiQrView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/WifiQrView.java @@ -121,7 +121,7 @@ public class WifiQrView extends ScrollView implements SwapWorkflowActivity.Inner private void setUIFromWifi() { - if (TextUtils.isEmpty(FDroidApp.REPO.address)) { + if (TextUtils.isEmpty(FDroidApp.repo.address)) { return; } @@ -139,7 +139,7 @@ public class WifiQrView extends ScrollView implements SwapWorkflowActivity.Inner * wifi AP to join. Lots of QR Scanners are buggy and do not respect * custom URI schemes, so we have to use http:// or https:// :-( */ - Uri sharingUri = Utils.getSharingUri(FDroidApp.REPO); + Uri sharingUri = Utils.getSharingUri(FDroidApp.repo); String qrUriString = (scheme + sharingUri.getHost()).toUpperCase(Locale.ENGLISH); if (sharingUri.getPort() != 80) { qrUriString += ":" + sharingUri.getPort(); From 5bb73999ada767474b4af042552fd57ad85219e5 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 13 May 2016 12:49:55 +0200 Subject: [PATCH 4/4] more javadoc about how the download URL is used as a unique ID --- .../main/java/org/fdroid/fdroid/ProgressListener.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/src/main/java/org/fdroid/fdroid/ProgressListener.java b/app/src/main/java/org/fdroid/fdroid/ProgressListener.java index 915b64ee6..1e71373ad 100644 --- a/app/src/main/java/org/fdroid/fdroid/ProgressListener.java +++ b/app/src/main/java/org/fdroid/fdroid/ProgressListener.java @@ -7,6 +7,15 @@ import java.net.URL; * updates, APKs, etc). This also keeps this class pure Java so that classes * that use {@code ProgressListener} can be tested on the JVM, without requiring * an Android device or emulator. + *

+ * The full URL of a download is used as the unique identifier throughout + * F-Droid. I can take a few forms: + *

    + *
  • {@link URL} instances + *
  • {@link android.net.Uri} instances + *
  • {@code String} instances, i.e. {@link URL#toString()} + *
  • {@code int}s, i.e. {@link String#hashCode()} + *
*/ public interface ProgressListener {