From ca4f26fd7d31eb6b4eb628171fd82e884869ac6d Mon Sep 17 00:00:00 2001 From: Tad Date: Mon, 20 Sep 2021 12:59:47 -0400 Subject: [PATCH] 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 --- app/build.gradle | 4 +- .../us/spotco/malwarescanner/Database.java | 12 +- .../spotco/malwarescanner/MainActivity.java | 103 +++++++----------- .../spotco/malwarescanner/MalwareScanner.java | 33 ++++-- .../malwarescanner/MalwareScannerService.java | 18 +-- .../malwarescanner/SignatureDatabase.java | 4 +- .../java/us/spotco/malwarescanner/Utils.java | 67 ++++++------ 7 files changed, 107 insertions(+), 134 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index fab900c..97966b3 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -6,8 +6,8 @@ android { applicationId "us.spotco.malwarescanner" minSdkVersion 16 targetSdkVersion 29 - versionCode 75 - versionName "2.21" + versionCode 76 + versionName "2.22" resConfigs "en", "de", "es", "fr", "it", "pt", "ru" } buildTypes { diff --git a/app/src/main/java/us/spotco/malwarescanner/Database.java b/app/src/main/java/us/spotco/malwarescanner/Database.java index 8bb7a84..7ba31d9 100644 --- a/app/src/main/java/us/spotco/malwarescanner/Database.java +++ b/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 signatureDatabases = new HashSet<>(); + public final static ConcurrentLinkedQueue 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 signatureDatabases) { + public static void updateDatabase(Context context, ConcurrentLinkedQueue 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 signatureDatabases) { + public static void loadDatabase(Context context, boolean forceReload, ConcurrentLinkedQueue signatureDatabases) { if ((!isDatabaseLoaded() || forceReload) && !databaseCurrentlyLoading) { databaseFullyLoaded = false; databaseCurrentlyLoading = true; diff --git a/app/src/main/java/us/spotco/malwarescanner/MainActivity.java b/app/src/main/java/us/spotco/malwarescanner/MainActivity.java index 754fc51..a4a4275 100644 --- a/app/src/main/java/us/spotco/malwarescanner/MainActivity.java +++ b/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,35 +88,30 @@ 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() { - @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() { - @Override - public void run() { - while (malwareScanner.running) { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - e.printStackTrace(); - } - } - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - 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; + fab.setOnClickListener(view -> { + if (!malwareScanner.running) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + fab.setBackgroundTintList(ColorStateList.valueOf(getColor(R.color.red))); } + startScanner(); + //TODO: This might not receive an available thread in time + Utils.getThreadPoolExecutor().execute(() -> { + while (malwareScanner.running) { + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + 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; } }); @@ -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() { - @Override - public void onClick(DialogInterface dialog, int which) { - //do nothing - } + creditsBuilder.setItems(R.array.fullCredits, (dialog, 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() { - @Override - 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(); - } + databaseBuilder.setMultiChoiceItems(databases, databaseDefaults, (dialogInterface, i, selected) -> databaseDefaults[i] = selected); + databaseBuilder.setPositiveButton("OK", (dialogInterface, 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 filesToScan = new HashSet<>(); + malwareScanner.running = true; + HashSet 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() { - @Override - public void run() { - Database.loadDatabase(getApplicationContext(), true, Database.signatureDatabases); - } - }); + Utils.getThreadPoolExecutor().execute(() -> Database.loadDatabase(getApplicationContext(), true, Database.signatureDatabases)); } } diff --git a/app/src/main/java/us/spotco/malwarescanner/MalwareScanner.java b/app/src/main/java/us/spotco/malwarescanner/MalwareScanner.java index c60a39c..9830d27 100644 --- a/app/src/main/java/us/spotco/malwarescanner/MalwareScanner.java +++ b/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, Object, String> { +class MalwareScanner extends AsyncTask, 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, Object, String> { } @Override - protected final String doInBackground(Set[] filesToScan) { + protected final String doInBackground(HashSet[] filesToScan) { running = true; + ConcurrentSkipListSet filesToScanReal = new ConcurrentSkipListSet<>(); //TODO: Reduce this? + for (Set 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, 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, 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, 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, Object, String> { fileHashesMD5.clear(); fileHashesSHA1.clear(); fileHashesSHA256.clear(); - Utils.FILES_SCANNED += filesToScan[0].size(); - if(userFacing || Utils.FILES_SCANNED % 40 == 0) { + Utils.FILES_SCANNED.getAndAdd(fileScannedCount); + 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 MBS = totalBytesHashed / 1000 / 1000 / secondsSpentHasing; + long secondsSpentHashing = ((SystemClock.elapsedRealtime() - hashStartTime) / 1000L); + long MBS = totalBytesHashed / 1000 / 1000 / secondsSpentHashing; publishProgress(context.getString(R.string.main_scanning_done, secondsSpent + "", MBS + "") + "\n\n\n\n", true); } } else { diff --git a/app/src/main/java/us/spotco/malwarescanner/MalwareScannerService.java b/app/src/main/java/us/spotco/malwarescanner/MalwareScannerService.java index e7a6972..6cdf3a4 100644 --- a/app/src/main/java/us/spotco/malwarescanner/MalwareScannerService.java +++ b/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()); - threadPoolExecutor.execute(new Runnable() { - @Override - public void run() { - Database.loadDatabase(getApplicationContext(), false, Database.signatureDatabases); - } - }); + int threadCount = Utils.getMaxThreads() + malwareMonitors.size(); + threadPoolExecutor = Utils.getNewThreadPoolExecutor(threadCount); + threadPoolExecutor.execute(() -> Database.loadDatabase(getApplicationContext(), false, Database.signatureDatabases)); for (final RecursiveFileObserver malwareMonitor : malwareMonitors) { - threadPoolExecutor.execute(new Runnable() { - @Override - public void run() { - malwareMonitor.startWatching(); - } - }); + threadPoolExecutor.execute(malwareMonitor::startWatching); } notificationManager = (NotificationManager) getApplicationContext().getSystemService(Context.NOTIFICATION_SERVICE); diff --git a/app/src/main/java/us/spotco/malwarescanner/SignatureDatabase.java b/app/src/main/java/us/spotco/malwarescanner/SignatureDatabase.java index 225c9a8..d647817 100644 --- a/app/src/main/java/us/spotco/malwarescanner/SignatureDatabase.java +++ b/app/src/main/java/us/spotco/malwarescanner/SignatureDatabase.java @@ -19,8 +19,8 @@ package us.spotco.malwarescanner; class SignatureDatabase { - private String url = null; - private String name = null; + private final String url; + private final String name; public SignatureDatabase(String url) { this.url = url; diff --git a/app/src/main/java/us/spotco/malwarescanner/Utils.java b/app/src/main/java/us/spotco/malwarescanner/Utils.java index 1147331..7bb4639 100644 --- a/app/src/main/java/us/spotco/malwarescanner/Utils.java +++ b/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,43 +40,50 @@ 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 ThreadPoolExecutor getNewThreadPoolExecutor(int threads) { + return new ThreadPoolExecutor(threads, threads, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(16), new ThreadPoolExecutor.CallerRunsPolicy()); + } + public static int getMaxThreads() { - int maxTheads = Runtime.getRuntime().availableProcessors(); - if (maxTheads > 4) { - maxTheads = 4; + int maxThreads = Runtime.getRuntime().availableProcessors(); + if (maxThreads > 4) { + maxThreads = 4; } - return maxTheads; + if(maxThreads < 2) { + maxThreads = 2; + } + return maxThreads; } public static HashSet getFilesRecursive(File root) { HashSet filesAll = new HashSet<>(); - - File[] files = root.listFiles(); - if (files != null && files.length > 0) { - for (File f : files) { - if (f.isDirectory()) { - HashSet filesTmp = 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); + 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()) { + filesAll.addAll(getFilesRecursive(f)); + } else { + if (f.length() <= MAX_SCAN_SIZE && f.canRead()) {//Exclude files larger than limit for performance + filesAll.add(f); + } } } } } - 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 { - s = new Socket(host, port); + try (Socket s = new Socket(host, port)) { + s.close(); 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 (!(listening = isPortListening("127.0.0.1", 9050)) && tries <= 60) { + while (!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() {