Create the HTTP client lazily and allow it to be garbage collected.

briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java

@@ -44,7 +44,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.security.GeneralSecurityException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.Date;
 import java.util.List;
@@ -55,15 +54,13 @@ import java.util.logging.Logger;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 import javax.inject.Inject;
-import javax.net.SocketFactory;
 
-import okhttp3.Dns;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
 import okhttp3.ResponseBody;
 
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.Collections.sort;
 import static java.util.logging.Level.WARNING;
 import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.briar.api.blog.BlogConstants.MAX_BLOG_POST_TEXT_LENGTH;
@@ -83,8 +80,6 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook,
 	private static final Logger LOG =
 			Logger.getLogger(FeedManagerImpl.class.getName());
 
-	private static final int CONNECT_TIMEOUT = 60 * 1000; // Milliseconds
-
 	private final TaskScheduler scheduler;
 	private final Executor ioExecutor;
 	private final DatabaseComponent db;
@@ -94,7 +89,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook,
 	private final BlogPostFactory blogPostFactory;
 	private final FeedFactory feedFactory;
 	private final Clock clock;
-	private final OkHttpClient client;
+	private final WeakSingletonProvider<OkHttpClient> httpClientProvider;
 	private final AtomicBoolean fetcherStarted = new AtomicBoolean(false);
 
 	private volatile boolean torActive = false;
@@ -108,9 +103,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook,
 			BlogManager blogManager,
 			BlogPostFactory blogPostFactory,
 			FeedFactory feedFactory,
-			SocketFactory torSocketFactory,
-			Clock clock,
-			Dns noDnsLookups) {
+			WeakSingletonProvider<OkHttpClient> httpClientProvider,
+			Clock clock) {
 		this.scheduler = scheduler;
 		this.ioExecutor = ioExecutor;
 		this.db = db;
@@ -119,14 +113,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook,
 		this.blogManager = blogManager;
 		this.blogPostFactory = blogPostFactory;
 		this.feedFactory = feedFactory;
+		this.httpClientProvider = httpClientProvider;
 		this.clock = clock;
-		// Share an HTTP client instance between all requests - see
-		// https://medium.com/@leandromazzuquini/if-you-are-using-okhttp-you-should-know-this-61d68e065a2b
-		client = new OkHttpClient.Builder()
-				.socketFactory(torSocketFactory)
-				.dns(noDnsLookups) // Don't make local DNS lookups
-				.connectTimeout(CONNECT_TIMEOUT, MILLISECONDS)
-				.build();
 	}
 
 	@Override
@@ -374,6 +362,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook,
 				.build();
 
 		// Execute Request
+		OkHttpClient client = httpClientProvider.get();
 		Response response = client.newCall(request).execute();
 		ResponseBody body = response.body();
 		if (body != null) return body.byteStream();
@@ -396,7 +385,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook,
 		long lastEntryTime = feed.getLastEntryTime();
 		Transaction txn = db.startTransaction(false);
 		try {
-			Collections.sort(entries, getEntryComparator());
+			//noinspection Java8ListSort
+			sort(entries, getEntryComparator());
 			for (SyndEntry entry : entries) {
 				long entryTime;
 				if (entry.getPublishedDate() != null) {

briar-core/src/main/java/org/briarproject/briar/feed/FeedModule.java

@@ -5,15 +5,23 @@ import org.briarproject.bramble.api.lifecycle.LifecycleManager;
 import org.briarproject.briar.api.blog.BlogManager;
 import org.briarproject.briar.api.feed.FeedManager;
 
+import javax.annotation.Nonnull;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import javax.net.SocketFactory;
 
 import dagger.Module;
 import dagger.Provides;
+import okhttp3.Dns;
+import okhttp3.OkHttpClient;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 @Module
 public class FeedModule {
 
+	private static final int CONNECT_TIMEOUT = 60_000; // Milliseconds
+
 	public static class EagerSingletons {
 		@Inject
 		FeedManager feedManager;
@@ -35,4 +43,25 @@ public class FeedModule {
 		return feedFactory;
 	}
 
+	// Share an HTTP client instance between requests where possible, while
+	// allowing the client to be garbage-collected between requests. The
+	// provider keeps a weak reference to the last client instance and reuses
+	// the instance until it gets garbage-collected. See
+	// https://medium.com/@leandromazzuquini/if-you-are-using-okhttp-you-should-know-this-61d68e065a2b
+	@Provides
+	@Singleton
+	WeakSingletonProvider<OkHttpClient> provideOkHttpClientProvider(
+			SocketFactory torSocketFactory, Dns noDnsLookups) {
+		return new WeakSingletonProvider<OkHttpClient>() {
+			@Override
+			@Nonnull
+			public OkHttpClient createInstance() {
+				return new OkHttpClient.Builder()
+						.socketFactory(torSocketFactory)
+						.dns(noDnsLookups) // Don't make local DNS lookups
+						.connectTimeout(CONNECT_TIMEOUT, MILLISECONDS)
+						.build();
+			}
+		};
+	}
 }

briar-core/src/main/java/org/briarproject/briar/feed/WeakSingletonProvider.java

@@ -0,0 +1,35 @@
+package org.briarproject.briar.feed;
+
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+import java.lang.ref.WeakReference;
+
+import javax.annotation.concurrent.GuardedBy;
+import javax.inject.Provider;
+
+/**
+ * A {@link Provider} that keeps a {@link WeakReference} to the last provided
+ * instance and provides the same instance again until the instance is garbage
+ * collected.
+ */
+@NotNullByDefault
+abstract class WeakSingletonProvider<T> implements Provider<T> {
+
+	private final Object lock = new Object();
+	@GuardedBy("lock")
+	private WeakReference<T> ref = new WeakReference<>(null);
+
+	@Override
+	public T get() {
+		synchronized (lock) {
+			T instance = ref.get();
+			if (instance == null) {
+				instance = createInstance();
+				ref = new WeakReference<>(instance);
+			}
+			return instance;
+		}
+	}
+
+	abstract T createInstance();
+}

briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java

@@ -33,10 +33,13 @@ import java.util.Date;
 import java.util.List;
 import java.util.concurrent.Executor;
 
+import javax.annotation.Nonnull;
 import javax.net.SocketFactory;
 
 import okhttp3.Dns;
+import okhttp3.OkHttpClient;
 
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.briarproject.bramble.test.TestUtils.getGroup;
 import static org.briarproject.bramble.test.TestUtils.getLocalAuthor;
 import static org.briarproject.bramble.test.TestUtils.getMessage;
@@ -59,6 +62,19 @@ public class FeedManagerImplTest extends BrambleMockTestCase {
 	private final Clock clock = context.mock(Clock.class);
 	private final Dns noDnsLookups = context.mock(Dns.class);
 
+	private final OkHttpClient client = new OkHttpClient.Builder()
+			.socketFactory(SocketFactory.getDefault())
+			.dns(noDnsLookups)
+			.connectTimeout(60_000, MILLISECONDS)
+			.build();
+	private final WeakSingletonProvider<OkHttpClient> httpClientProvider =
+			new WeakSingletonProvider<OkHttpClient>() {
+				@Override
+				@Nonnull
+				OkHttpClient createInstance() {
+					return client;
+				}
+			};
 	private final Group localGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
 	private final GroupId localGroupId = localGroup.getId();
 	private final Group blogGroup =
@@ -73,7 +89,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase {
 	private final FeedManagerImpl feedManager =
 			new FeedManagerImpl(scheduler, ioExecutor, db, contactGroupFactory,
 					clientHelper, blogManager, blogPostFactory, feedFactory,
-					SocketFactory.getDefault(), clock, noDnsLookups);
+					httpClientProvider, clock);
 
 	@Test
 	public void testFetchFeedsReturnsEarlyIfTorIsNotActive() {