Many changes

- Bump version
- Don't find files recursively on main thread
- Reduce number of threadpools
- Ensure there are always enough threads available
- Cap the queue limit on thread pools
- Do work on main thread if queues are full, thereby blocking
- Use more thread safe objects
- Replace some runnables with lambas
- Code cleanup
- Typo fixes

Closes https://gitlab.com/divested-mobile/hypatia/-/issues/6
Closes https://github.com/Divested-Mobile/Hypatia/issues/13
Closes https://github.com/Divested-Mobile/Hypatia/issues/1

Signed-off-by: Tad <tad@spotco.us>

app/build.gradle

@@ -6,8 +6,8 @@ android {
         applicationId "us.spotco.malwarescanner"
         minSdkVersion 16
         targetSdkVersion 29
-        versionCode 75
+        versionCode 76
-        versionName "2.21"
+        versionName "2.22"
         resConfigs "en", "de", "es", "fr", "it", "pt", "ru"
     }
     buildTypes {

app/src/main/java/us/spotco/malwarescanner/Database.java

@@ -35,7 +35,7 @@ import java.net.URL;
 import java.text.DateFormat;
 import java.util.Date;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.zip.GZIPInputStream;
@@ -45,11 +45,10 @@ class Database {
     private static TextView log = null;
     private static SharedPreferences prefs = null;
     private static File databasePath = null;
-    private static ThreadPoolExecutor threadPoolExecutor = null;
     private static boolean databaseFullyLoaded = false;
     private static boolean databaseCurrentlyLoading = false;
 
-    public final static HashSet<SignatureDatabase> signatureDatabases = new HashSet<>();
+    public final static ConcurrentLinkedQueue<SignatureDatabase> signatureDatabases = new ConcurrentLinkedQueue<>();
     public final static String baseURL = "https://divested.dev/MalwareScannerSignatures/";
     public final static String baseURLOnion = "https://hypatiagbf5vp3ba.onion/MalwareScannerSignatures/"; //TODO: Setup the .onion
 
@@ -61,7 +60,6 @@ class Database {
 
     public Database(TextView log) {
         Database.log = log;
-        threadPoolExecutor = (ThreadPoolExecutor) Executors.newScheduledThreadPool(Utils.getMaxThreads());
     }
 
     public static boolean areDatabasesAvailable() {
@@ -76,12 +74,12 @@ class Database {
         return signaturesMD5.size() + signaturesSHA1.size() + signaturesSHA256.size();
     }
 
-    public static void updateDatabase(Context context, HashSet<SignatureDatabase> signatureDatabases) {
+    public static void updateDatabase(Context context, ConcurrentLinkedQueue<SignatureDatabase> signatureDatabases) {
         initDatabase(context);
         log.append(context.getString(R.string.main_database_updating, signatureDatabases.size() + "") + "\n");
         for (SignatureDatabase signatureDatabase : signatureDatabases) {
             boolean onionRouting = prefs.getBoolean("ONION_ROUTING", false);
-            new Downloader().executeOnExecutor(threadPoolExecutor, onionRouting, signatureDatabase.getUrl(), databasePath + "/" + signatureDatabase.getName());
+            new Downloader().executeOnExecutor(Utils.getThreadPoolExecutor(), onionRouting, signatureDatabase.getUrl(), databasePath + "/" + signatureDatabase.getName());
         }
     }
 
@@ -114,7 +112,7 @@ class Database {
         }
     }
 
-    public static void loadDatabase(Context context, boolean forceReload, HashSet<SignatureDatabase> signatureDatabases) {
+    public static void loadDatabase(Context context, boolean forceReload, ConcurrentLinkedQueue<SignatureDatabase> signatureDatabases) {
         if ((!isDatabaseLoaded() || forceReload) && !databaseCurrentlyLoading) {
             databaseFullyLoaded = false;
             databaseCurrentlyLoading = true;

app/src/main/java/us/spotco/malwarescanner/MainActivity.java

@@ -21,7 +21,6 @@ import android.Manifest;
 import android.app.AlertDialog;
 import android.app.Dialog;
 import android.content.Context;
-import android.content.DialogInterface;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.content.pm.ApplicationInfo;
@@ -33,7 +32,6 @@ import android.os.Environment;
 import android.text.method.ScrollingMovementMethod;
 import android.view.Menu;
 import android.view.MenuItem;
-import android.view.View;
 import android.view.WindowManager;
 import android.widget.TextView;
 import android.widget.Toast;
@@ -48,7 +46,6 @@ import com.google.android.material.floatingactionbutton.FloatingActionButton;
 
 import java.io.File;
 import java.util.HashSet;
-import java.util.Set;
 
 public class MainActivity extends AppCompatActivity {
 
@@ -91,17 +88,14 @@ public class MainActivity extends AppCompatActivity {
         prefs = getSharedPreferences(BuildConfig.APPLICATION_ID, Context.MODE_PRIVATE);
 
         final FloatingActionButton fab = findViewById(R.id.fab);
-        fab.setOnClickListener(new View.OnClickListener() {
+        fab.setOnClickListener(view -> {
-            @Override
-            public void onClick(View view) {
             if (!malwareScanner.running) {
                 if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
                     fab.setBackgroundTintList(ColorStateList.valueOf(getColor(R.color.red)));
                 }
                 startScanner();
-                    Utils.getThreadPoolExecutor().execute(new Runnable() {
+                //TODO: This might not receive an available thread in time
-                        @Override
+                Utils.getThreadPoolExecutor().execute(() -> {
-                        public void run() {
                     while (malwareScanner.running) {
                         try {
                             Thread.sleep(1000);
@@ -113,14 +107,12 @@ public class MainActivity extends AppCompatActivity {
                         fab.setBackgroundTintList(ColorStateList.valueOf(getColor(R.color.light_blue)));
                     }
 
-                        }
                 });
             } else {
                 logView.append("\n" + getString(R.string.main_cancelling_scan) + "\n\n");
                 malwareScanner.cancel(true);
                 malwareScanner.running = false;
             }
-            }
         });
 
         requestPermissions();
@@ -146,18 +138,15 @@ public class MainActivity extends AppCompatActivity {
         Dialog creditsDialog;
         AlertDialog.Builder creditsBuilder = new AlertDialog.Builder(this);
         creditsBuilder.setTitle(getString(R.string.lblFullCredits));
-        creditsBuilder.setItems(R.array.fullCredits, new DialogInterface.OnClickListener() {
+        creditsBuilder.setItems(R.array.fullCredits, (dialog, which) -> {
-            @Override
-            public void onClick(DialogInterface dialog, int which) {
             //do nothing
-            }
         });
         creditsDialog = creditsBuilder.create();
         creditsDialog.show();
     }
 
     private String localizeDBDescription(String desc) {
-        String localDesc = desc
+        return desc
                 .replaceAll("AUTHOR", getString(R.string.db_desc_author))
                 .replaceAll("LICENSE", getString(R.string.db_desc_license))
                 .replaceAll("SIZE_SMALL", getString(R.string.db_desc_size_small))
@@ -165,7 +154,6 @@ public class MainActivity extends AppCompatActivity {
                 .replaceAll("SIZE_LARGE", getString(R.string.db_desc_size_large))
                 .replaceAll("SIZE", getString(R.string.db_desc_size))
                 .replaceAll("SOURCE", getString(R.string.db_desc_source));
-        return localDesc;
     }
 
     private void selectDatabases() {
@@ -186,21 +174,13 @@ public class MainActivity extends AppCompatActivity {
         AlertDialog.Builder databaseBuilder = new AlertDialog.Builder(this);
         databaseBuilder.setTitle(R.string.lblSelectDatabasesTitle);
 
-        databaseBuilder.setMultiChoiceItems(databases, databaseDefaults, new DialogInterface.OnMultiChoiceClickListener() {
+        databaseBuilder.setMultiChoiceItems(databases, databaseDefaults, (dialogInterface, i, selected) -> databaseDefaults[i] = selected);
-            @Override
+        databaseBuilder.setPositiveButton("OK", (dialogInterface, i) -> {
-            public void onClick(DialogInterface dialogInterface, int i, boolean selected) {
-                databaseDefaults[i] = selected;
-            }
-        });
-        databaseBuilder.setPositiveButton("OK", new DialogInterface.OnClickListener() {
-            @Override
-            public void onClick(DialogInterface dialogInterface, int i) {
             prefs.edit().putBoolean("SIGNATURES_CLAMAV-ANDROID", databaseDefaults[0]).apply();
             prefs.edit().putBoolean("SIGNATURES_CLAMAV-MAIN", databaseDefaults[1]).apply();
             prefs.edit().putBoolean("SIGNATURES_CLAMAV-DAILY", databaseDefaults[2]).apply();
             prefs.edit().putBoolean("SIGNATURES_ESET", databaseDefaults[3]).apply();
             prefs.edit().putBoolean("SIGNATURES_TARGETEDTHREATS", databaseDefaults[4]).apply();
-            }
         });
 
         databaseDialog = databaseBuilder.create();
@@ -271,9 +251,10 @@ public class MainActivity extends AppCompatActivity {
 
     private void startScanner() {
         malwareScanner = new MalwareScanner(this, this, true);
-        Set<File> filesToScan = new HashSet<>();
+        malwareScanner.running = true;
+        HashSet<File> filesToScan = new HashSet<>();
         if (scanSystem) {
-            filesToScan.addAll(Utils.getFilesRecursive(Environment.getRootDirectory()));
+            filesToScan.add(Environment.getRootDirectory());
         }
         if (scanApps) {
             for (ApplicationInfo packageInfo : getPackageManager().getInstalledApplications(PackageManager.GET_META_DATA)) {
@@ -281,25 +262,19 @@ public class MainActivity extends AppCompatActivity {
             }
         }
         if (scanInternal) {
-            filesToScan.addAll(Utils.getFilesRecursive(Environment.getExternalStorageDirectory()));
+            filesToScan.add(Environment.getExternalStorageDirectory());
         }
         if (scanExternal) {
-            filesToScan.addAll(Utils.getFilesRecursive(new File("/storage")));
+            filesToScan.add(new File("/storage"));
         }
         malwareScanner.executeOnExecutor(Utils.getThreadPoolExecutor(), filesToScan);
-        malwareScanner.running = true;
     }
 
     private void updateDatabase() {
         new Database((TextView) findViewById(R.id.txtLogOutput));
         Database.updateDatabase(this, Database.signatureDatabases);
         if (Database.isDatabaseLoaded()) {
-            Utils.getThreadPoolExecutor().execute(new Runnable() {
+            Utils.getThreadPoolExecutor().execute(() -> Database.loadDatabase(getApplicationContext(), true, Database.signatureDatabases));
-                @Override
-                public void run() {
-                    Database.loadDatabase(getApplicationContext(), true, Database.signatureDatabases);
-                }
-            });
         }
     }
 

app/src/main/java/us/spotco/malwarescanner/MalwareScanner.java

@@ -37,13 +37,15 @@ import java.math.BigInteger;
 import java.security.MessageDigest;
 import java.text.NumberFormat;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
+import java.util.concurrent.ConcurrentSkipListSet;
 
-class MalwareScanner extends AsyncTask<Set<File>, Object, String> {
+class MalwareScanner extends AsyncTask<HashSet<File>, Object, String> {
 
-    private Context context = null;
+    private final Context context;
     private TextView logOutput = null;
     private boolean userFacing = false;
     private NotificationManager notificationManager = null;
@@ -100,15 +102,22 @@ class MalwareScanner extends AsyncTask<Set<File>, Object, String> {
     }
 
     @Override
-    protected final String doInBackground(Set<File>[] filesToScan) {
+    protected final String doInBackground(HashSet<File>[] filesToScan) {
         running = true;
+        ConcurrentSkipListSet<File> filesToScanReal = new ConcurrentSkipListSet<>(); //TODO: Reduce this?
+        for (Set<File> fileArray : filesToScan) {
+            for (File file : fileArray) {
+                filesToScanReal.addAll(Utils.getFilesRecursive(file)); //TODO: Inline this, hash files as they are found
+            }
+        }
+        filesToScan = null;
 
         //Pre
         fileHashesMD5.clear();
         fileHashesSHA1.clear();
         fileHashesSHA256.clear();
 
-        publishProgress("\t" + context.getString(R.string.main_files_pending_scan, NumberFormat.getInstance().format(filesToScan[0].size()) + "") + "\n", true);
+        publishProgress("\t" + context.getString(R.string.main_files_pending_scan, NumberFormat.getInstance().format(filesToScanReal.size()) + "") + "\n", true);
 
         Database.loadDatabase(context, false, Database.signatureDatabases);
         int delayCount = 0;
@@ -132,14 +141,14 @@ class MalwareScanner extends AsyncTask<Set<File>, Object, String> {
             publishProgress("\t" + context.getString(R.string.main_hashing_files), true);
             publishProgress("\t", true);
             int fileScannedCount = 0;
-            int percentIncrement = (filesToScan[0].size() / 20);
+            int percentIncrement = (filesToScanReal.size() / 20);
             if (percentIncrement < 1) { //Prevent divide by zero
                 percentIncrement = 1;
             }
             String spinnerCur = " ~ ";
             long totalBytesHashed = 0;
             long hashStartTime = SystemClock.elapsedRealtime();
-            for (File file : filesToScan[0]) {
+            for (File file : filesToScanReal) {
                 if (this.isCancelled()) { //Allow quicker cancels
                     //publishProgress("\t" + context.getString(R.string.main_cancelled_scan), true);
                     running = false;
@@ -147,6 +156,7 @@ class MalwareScanner extends AsyncTask<Set<File>, Object, String> {
                 }
                 totalBytesHashed += file.length();
                 getFileHashes(file);
+                filesToScanReal.remove(file);
                 fileScannedCount++;
                 if ((fileScannedCount % percentIncrement) == 0) {
                     publishProgress(spinnerCur, true);
@@ -157,6 +167,7 @@ class MalwareScanner extends AsyncTask<Set<File>, Object, String> {
                     }
                 }
             }
+            filesToScanReal.clear();
             publishProgress(" !\n\t" + context.getString(R.string.main_hashing_done) + "\n", true);
 
             //Check the hashes
@@ -168,14 +179,14 @@ class MalwareScanner extends AsyncTask<Set<File>, Object, String> {
             fileHashesMD5.clear();
             fileHashesSHA1.clear();
             fileHashesSHA256.clear();
-            Utils.FILES_SCANNED += filesToScan[0].size();
+            Utils.FILES_SCANNED.getAndAdd(fileScannedCount);
-            if(userFacing || Utils.FILES_SCANNED % 40 == 0) {
+            if (userFacing || Utils.FILES_SCANNED.get() % 40 == 0) {
                 System.gc(); //GC can be expensive, don't run it too often.
             }
-            if(userFacing) {
+            if (userFacing) {
                 long secondsSpent = ((SystemClock.elapsedRealtime() - scanStartTime) / 1000L);
-                long secondsSpentHasing = ((SystemClock.elapsedRealtime() - hashStartTime) / 1000L);
+                long secondsSpentHashing = ((SystemClock.elapsedRealtime() - hashStartTime) / 1000L);
-                long MBS = totalBytesHashed / 1000 / 1000 / secondsSpentHasing;
+                long MBS = totalBytesHashed / 1000 / 1000 / secondsSpentHashing;
                 publishProgress(context.getString(R.string.main_scanning_done, secondsSpent + "", MBS + "") + "\n\n\n\n", true);
             }
         } else {

app/src/main/java/us/spotco/malwarescanner/MalwareScannerService.java

@@ -34,7 +34,6 @@ import androidx.core.app.NotificationCompat;
 import java.io.File;
 import java.text.NumberFormat;
 import java.util.HashSet;
-import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadPoolExecutor;
 
 public class MalwareScannerService extends Service {
@@ -55,21 +54,12 @@ public class MalwareScannerService extends Service {
         malwareMonitors.clear();
         addMalwareMonitor(Environment.getExternalStorageDirectory().toString());
 
-        threadPoolExecutor = (ThreadPoolExecutor) Executors.newScheduledThreadPool(Utils.getMaxThreads() + malwareMonitors.size());
+        int threadCount = Utils.getMaxThreads() + malwareMonitors.size();
-        threadPoolExecutor.execute(new Runnable() {
+        threadPoolExecutor = Utils.getNewThreadPoolExecutor(threadCount);
-            @Override
+        threadPoolExecutor.execute(() -> Database.loadDatabase(getApplicationContext(), false, Database.signatureDatabases));
-            public void run() {
-                Database.loadDatabase(getApplicationContext(), false, Database.signatureDatabases);
-            }
-        });
 
         for (final RecursiveFileObserver malwareMonitor : malwareMonitors) {
-            threadPoolExecutor.execute(new Runnable() {
+            threadPoolExecutor.execute(malwareMonitor::startWatching);
-                @Override
-                public void run() {
-                    malwareMonitor.startWatching();
-                }
-            });
         }
 
         notificationManager = (NotificationManager) getApplicationContext().getSystemService(Context.NOTIFICATION_SERVICE);

app/src/main/java/us/spotco/malwarescanner/SignatureDatabase.java

@@ -19,8 +19,8 @@ package us.spotco.malwarescanner;
 
 class SignatureDatabase {
 
-    private String url = null;
+    private final String url;
-    private String name = null;
+    private final String name;
 
     public SignatureDatabase(String url) {
         this.url = url;

app/src/main/java/us/spotco/malwarescanner/Utils.java

@@ -27,8 +27,10 @@ import android.os.Build;
 import java.io.File;
 import java.net.Socket;
 import java.util.HashSet;
-import java.util.concurrent.Executors;
+import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
 class Utils {
 
@@ -38,35 +40,42 @@ class Utils {
 
     public final static int MAX_HASH_LENGTH = 12;
 
-    public static int FILES_SCANNED = 0;
+    public static final AtomicInteger FILES_SCANNED = new AtomicInteger();
     private static ThreadPoolExecutor threadPoolExecutor = null;
 
     public static ThreadPoolExecutor getThreadPoolExecutor() {
         if (threadPoolExecutor == null) {
-            threadPoolExecutor = (ThreadPoolExecutor) Executors.newScheduledThreadPool(getMaxThreads());
+            threadPoolExecutor = getNewThreadPoolExecutor(getMaxThreads());
         }
         return threadPoolExecutor;
     }
 
-    public static int getMaxThreads() {
+    public static ThreadPoolExecutor getNewThreadPoolExecutor(int threads) {
-        int maxTheads = Runtime.getRuntime().availableProcessors();
+        return new ThreadPoolExecutor(threads, threads, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(16), new ThreadPoolExecutor.CallerRunsPolicy());
-        if (maxTheads > 4) {
-            maxTheads = 4;
     }
-        return maxTheads;
+
+    public static int getMaxThreads() {
+        int maxThreads = Runtime.getRuntime().availableProcessors();
+        if (maxThreads > 4) {
+            maxThreads = 4;
+        }
+        if(maxThreads < 2) {
+            maxThreads = 2;
+        }
+        return maxThreads;
     }
 
     public static HashSet<File> getFilesRecursive(File root) {
         HashSet<File> filesAll = new HashSet<>();
-
+        if (root.isFile()) { //TODO: Skip this
+            filesAll.add(root);
+            return filesAll;
+        } else {
             File[] files = root.listFiles();
             if (files != null && files.length > 0) {
                 for (File f : files) {
                     if (f.isDirectory()) {
-                    HashSet<File> filesTmp = getFilesRecursive(f);
+                        filesAll.addAll(getFilesRecursive(f));
-                    if (filesTmp != null) {
-                        filesAll.addAll(filesTmp);
-                    }
                     } else {
                         if (f.length() <= MAX_SCAN_SIZE && f.canRead()) {//Exclude files larger than limit for performance
                             filesAll.add(f);
@@ -74,7 +83,7 @@ class Utils {
                     }
                 }
             }
-
+        }
         return filesAll;
     }
 
@@ -130,26 +139,17 @@ class Utils {
 
     //Credit: https://www.geekality.net/2013/04/30/java-simple-check-to-see-if-a-server-is-listening-on-a-port/
     public static boolean isPortListening(String host, int port) {
-        Socket s = null;
+        try (Socket s = new Socket(host, port)) {
-        try {
+            s.close();
-            s = new Socket(host, port);
             return true;
         } catch (Exception e) {
             return false;
-        } finally {
-            if (s != null) {
-                try {
-                    s.close();
-                } catch (Exception e1) {
-                }
-            }
         }
     }
 
-    public static boolean waitUntilOrbotIsAvailable() {
+    public static void waitUntilOrbotIsAvailable() {
         int tries = 0;
-        boolean listening;
+        while (!isPortListening("127.0.0.1", 9050) && tries <= 60) {
-        while (!(listening = isPortListening("127.0.0.1", 9050)) && tries <= 60) {
             tries++;
             try {
                 Thread.sleep(1000);
@@ -157,7 +157,6 @@ class Utils {
 
             }
         }
-        return listening;
     }
 
     public static Context getContext() {