From 9ea4463cbc59cd0957ae4b2d664f3a731e04fe7c Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jul 2020 16:46:31 +0100 Subject: [PATCH] Avoid creating an in-memory copy of the log where possible. This helps to avoid OOMs on low-memory devices. --- .../api/logging/PersistentLogManager.java | 8 ++--- .../logging/PersistentLogManagerImpl.java | 36 ++++++------------- .../android/reporting/BriarReportPrimer.java | 15 ++++++-- .../android/settings/SettingsActivity.java | 13 +++---- 4 files changed, 35 insertions(+), 37 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/logging/PersistentLogManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/logging/PersistentLogManager.java index d3c81c33e..23b9059ca 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/logging/PersistentLogManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/logging/PersistentLogManager.java @@ -5,7 +5,7 @@ import org.briarproject.bramble.api.settings.Settings; import java.io.File; import java.io.IOException; -import java.util.List; +import java.util.Scanner; import java.util.logging.Handler; import java.util.logging.Logger; @@ -36,11 +36,11 @@ public interface PersistentLogManager { void addLogHandler(File dir, Logger logger) throws IOException; /** - * Loads and returns the persistent log entries stored in the given - * directory, or an empty list if no log entries are found. + * Returns a {@link Scanner} for reading the persistent log entries stored + * in the given directory. * * @param old True if the previous session's log should be loaded, or false * if the current session's log should be loaded */ - List getPersistedLog(File dir, boolean old) throws IOException; + Scanner getPersistedLog(File dir, boolean old) throws IOException; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/logging/PersistentLogManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/logging/PersistentLogManagerImpl.java index 4af59b262..0bf1bea4b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/logging/PersistentLogManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/logging/PersistentLogManagerImpl.java @@ -16,14 +16,13 @@ import org.briarproject.bramble.api.transport.StreamReaderFactory; import org.briarproject.bramble.api.transport.StreamWriter; import org.briarproject.bramble.api.transport.StreamWriterFactory; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.LinkedList; -import java.util.List; import java.util.Scanner; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; @@ -37,7 +36,6 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; -import static java.util.Collections.emptyList; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logException; @@ -52,7 +50,6 @@ class PersistentLogManagerImpl implements PersistentLogManager, private static final String LOG_FILE = "briar.log"; private static final String OLD_LOG_FILE = "briar.log.old"; - private static final int MAX_LINES_TO_RETURN = 10_000; private final ScheduledExecutorService scheduler; private final Executor ioExecutor; @@ -147,13 +144,13 @@ class PersistentLogManagerImpl implements PersistentLogManager, } @Override - public List getPersistedLog(File dir, boolean old) + public Scanner getPersistedLog(File dir, boolean old) throws IOException { if (old) { SecretKey oldLogKey = this.oldLogKey; if (oldLogKey == null) { LOG.info("Old log key has not been loaded"); - return emptyList(); + return emptyScanner(); } return getPersistedLog(new File(dir, OLD_LOG_FILE), oldLogKey); } else { @@ -161,31 +158,20 @@ class PersistentLogManagerImpl implements PersistentLogManager, } } - private List getPersistedLog(File logFile, SecretKey key) + private Scanner getPersistedLog(File logFile, SecretKey key) throws IOException { if (logFile.exists()) { LOG.info("Reading log file"); - LinkedList lines = new LinkedList<>(); - int numLines = 0; InputStream in = new FileInputStream(logFile); - //noinspection TryFinallyCanBeTryWithResources - try { - InputStream reader = streamReaderFactory - .createLogStreamReader(in, key); - Scanner s = new Scanner(reader); - while (s.hasNextLine()) { - lines.add(s.nextLine()); - if (numLines == MAX_LINES_TO_RETURN) lines.poll(); - else numLines++; - } - s.close(); - return lines; - } finally { - in.close(); - } + return new Scanner(streamReaderFactory.createLogStreamReader(in, + key)); } else { LOG.info("Log file does not exist"); - return emptyList(); + return emptyScanner(); } } + + private Scanner emptyScanner() { + return new Scanner(new ByteArrayInputStream(new byte[0])); + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/reporting/BriarReportPrimer.java b/briar-android/src/main/java/org/briarproject/briar/android/reporting/BriarReportPrimer.java index 21395b737..a8d74310f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/reporting/BriarReportPrimer.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/reporting/BriarReportPrimer.java @@ -26,7 +26,9 @@ import java.lang.reflect.Method; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.Map; +import java.util.Scanner; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.FutureTask; @@ -54,6 +56,8 @@ import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; public class BriarReportPrimer implements ReportPrimer { + private static final int MAX_PERSISTED_LOG_LINES = 1_000; + @Override public void primeReport(@NonNull Context ctx, @NonNull ReportBuilder builder) { @@ -271,9 +275,16 @@ public class BriarReportPrimer implements ReportPrimer { File logDir = ctx.getDir("log", MODE_PRIVATE); StringBuilder sb = new StringBuilder(); try { - for (String line : logManager.getPersistedLog(logDir, old)) { - sb.append(line).append('\n'); + Scanner scanner = logManager.getPersistedLog(logDir, old); + LinkedList lines = new LinkedList<>(); + int numLines = 0; + while (scanner.hasNextLine()) { + lines.add(scanner.nextLine()); + if (numLines == MAX_PERSISTED_LOG_LINES) lines.pollFirst(); + else numLines++; } + scanner.close(); + for (String line : lines) sb.append(line).append('\n'); } catch (IOException e) { sb.append("Could not recover persisted log: ").append(e); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsActivity.java index a2f9851ec..9ec0789f7 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsActivity.java @@ -19,7 +19,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; -import java.util.List; +import java.util.Scanner; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -106,18 +106,19 @@ public class SettingsActivity extends BriarActivity { ioExecutor.execute(() -> { try { File logDir = getApplication().getDir("log", MODE_PRIVATE); - List lines = logManager.getPersistedLog(logDir, old); - if (lines.isEmpty()) { + Scanner scanner = logManager.getPersistedLog(logDir, old); + if (!scanner.hasNextLine()) { + scanner.close(); runOnUiThreadUnlessDestroyed(() -> Toast.makeText(getApplication(), "Log is empty", LENGTH_LONG).show()); return; } PrintWriter w = new PrintWriter(osp.getOutputStream()); - for (String line : lines) { - w.println(line); - } + while (scanner.hasNextLine()) w.println(scanner.nextLine()); + w.flush(); w.close(); + scanner.close(); runOnUiThreadUnlessDestroyed(() -> Toast.makeText(getApplication(), "Log exported", LENGTH_LONG).show());