Skip to content

Commit a9015eb

Browse files
JakeWhartonJakeWharton
committed
Add a strict mode of sorts to InMemoryFileSystem to detect handle leaks.
1 parent 0c18517 commit a9015eb

File tree

6 files changed

+94
-24
lines changed

6 files changed

+94
-24
lines changed

‎okhttp-testing-support/src/main/java/com/squareup/okhttp/internal/io/InMemoryFileSystem.java

+70-7
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,95 @@
1818
import java.io.File;
1919
import java.io.FileNotFoundException;
2020
import java.io.IOException;
21+
import java.util.ArrayList;
22+
import java.util.IdentityHashMap;
2123
import java.util.Iterator;
2224
import java.util.LinkedHashMap;
25+
import java.util.List;
2326
import java.util.Map;
2427
import okio.Buffer;
28+
import okio.ForwardingSink;
29+
import okio.ForwardingSource;
2530
import okio.Sink;
2631
import okio.Source;
32+
import org.junit.rules.TestRule;
33+
import org.junit.runner.Description;
34+
import org.junit.runners.model.Statement;
2735

2836
/** A simple file system where all files are held in memory. Not safe for concurrent use. */
29-
public final class InMemoryFileSystem implements FileSystem {
37+
public final class InMemoryFileSystem implements FileSystem, TestRule {
3038
private final Map<File, Buffer> files = new LinkedHashMap<>();
39+
private final Map<Source, File> openSources = new IdentityHashMap<>();
40+
private final Map<Sink, File> openSinks = new IdentityHashMap<>();
41+
42+
@Override public Statement apply(final Statement base, Description description) {
43+
return new Statement() {
44+
@Override public void evaluate() throws Throwable {
45+
base.evaluate();
46+
ensureResourcesClosed();
47+
}
48+
};
49+
}
50+
51+
public void ensureResourcesClosed() {
52+
List<String> openResources = new ArrayList<>();
53+
for (File file : openSources.values()) {
54+
openResources.add("Source for " + file);
55+
}
56+
for (File file : openSinks.values()) {
57+
openResources.add("Sink for " + file);
58+
}
59+
if (!openResources.isEmpty()) {
60+
StringBuilder builder = new StringBuilder("Resources acquired but not closed:");
61+
for (String resource : openResources) {
62+
builder.append("\n * ").append(resource);
63+
}
64+
throw new IllegalStateException(builder.toString());
65+
}
66+
}
3167

3268
@Override public Source source(File file) throws FileNotFoundException {
3369
Buffer result = files.get(file);
3470
if (result == null) throw new FileNotFoundException();
35-
return result.clone();
71+
72+
final Source source = result.clone();
73+
openSources.put(source, file);
74+
75+
return new ForwardingSource(source) {
76+
@Override public void close() throws IOException {
77+
openSources.remove(source);
78+
super.close();
79+
}
80+
};
3681
}
3782

3883
@Override public Sink sink(File file) throws FileNotFoundException {
39-
Buffer result = new Buffer();
40-
files.put(file, result);
41-
return result;
84+
return sink(file, false);
4285
}
4386

4487
@Override public Sink appendingSink(File file) throws FileNotFoundException {
45-
Buffer result = files.get(file);
46-
return result != null ? result : sink(file);
88+
return sink(file, true);
89+
}
90+
91+
private Sink sink(File file, boolean appending) {
92+
Buffer result = null;
93+
if (appending) {
94+
result = files.get(file);
95+
}
96+
if (result == null) {
97+
result = new Buffer();
98+
}
99+
files.put(file, result);
100+
101+
final Sink sink = result;
102+
openSinks.put(sink, file);
103+
104+
return new ForwardingSink(sink) {
105+
@Override public void close() throws IOException {
106+
openSinks.remove(sink);
107+
super.close();
108+
}
109+
};
47110
}
48111

49112
@Override public void delete(File file) throws IOException {

‎okhttp-tests/src/test/java/com/squareup/okhttp/CacheTest.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.squareup.okhttp.internal.Internal;
2020
import com.squareup.okhttp.internal.SslContextBuilder;
2121
import com.squareup.okhttp.internal.Util;
22-
import com.squareup.okhttp.internal.io.FileSystem;
2322
import com.squareup.okhttp.internal.io.InMemoryFileSystem;
2423
import com.squareup.okhttp.mockwebserver.MockResponse;
2524
import com.squareup.okhttp.mockwebserver.MockWebServer;
@@ -76,9 +75,9 @@ public final class CacheTest {
7675

7776
@Rule public MockWebServer server = new MockWebServer();
7877
@Rule public MockWebServer server2 = new MockWebServer();
78+
@Rule public InMemoryFileSystem fileSystem = new InMemoryFileSystem();
7979

8080
private final SSLContext sslContext = SslContextBuilder.localhost();
81-
private final FileSystem fileSystem = new InMemoryFileSystem();
8281
private final OkHttpClient client = new OkHttpClient();
8382
private Cache cache;
8483
private final CookieManager cookieManager = new CookieManager();
@@ -93,6 +92,7 @@ public final class CacheTest {
9392
@After public void tearDown() throws Exception {
9493
ResponseCache.setDefault(null);
9594
CookieHandler.setDefault(null);
95+
cache.delete();
9696
}
9797

9898
/**
@@ -266,7 +266,7 @@ private void testResponseCaching(TransferKind transferKind) throws IOException {
266266
Principal localPrincipal = response1.handshake().localPrincipal();
267267

268268
Response response2 = client.newCall(request).execute(); // Cached!
269-
assertEquals("ABC", response2.body().source().readUtf8());
269+
assertEquals("ABC", response2.body().string());
270270

271271
assertEquals(2, cache.getRequestCount());
272272
assertEquals(1, cache.getNetworkCount());
@@ -1873,6 +1873,7 @@ public void assertCookies(HttpUrl url, String... expectedCookies) throws Excepti
18731873

18741874
Response response = get(server.url("/"));
18751875
assertEquals("A", response.header(""));
1876+
assertEquals("body", response.body().string());
18761877
}
18771878

18781879
/**

‎okhttp-tests/src/test/java/com/squareup/okhttp/CallTest.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.squareup.okhttp.internal.SslContextBuilder;
2323
import com.squareup.okhttp.internal.Util;
2424
import com.squareup.okhttp.internal.Version;
25-
import com.squareup.okhttp.internal.io.FileSystem;
2625
import com.squareup.okhttp.internal.io.InMemoryFileSystem;
2726
import com.squareup.okhttp.mockwebserver.Dispatcher;
2827
import com.squareup.okhttp.mockwebserver.MockResponse;
@@ -88,9 +87,9 @@ public final class CallTest {
8887
@Rule public final TestRule timeout = new Timeout(30_000);
8988
@Rule public final MockWebServer server = new MockWebServer();
9089
@Rule public final MockWebServer server2 = new MockWebServer();
90+
@Rule public final InMemoryFileSystem fileSystem = new InMemoryFileSystem();
9191

9292
private SSLContext sslContext = SslContextBuilder.localhost();
93-
private FileSystem fileSystem = new InMemoryFileSystem();
9493
private OkHttpClient client = new OkHttpClient();
9594
private RecordingCallback callback = new RecordingCallback();
9695
private TestLogHandler logHandler = new TestLogHandler();

‎okhttp-tests/src/test/java/com/squareup/okhttp/RecordingCallback.java

+2-6
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.Iterator;
2121
import java.util.List;
2222
import java.util.concurrent.TimeUnit;
23-
import okio.Buffer;
2423

2524
/**
2625
* Records received HTTP responses so they can be later retrieved by tests.
@@ -36,11 +35,8 @@ public class RecordingCallback implements Callback {
3635
}
3736

3837
@Override public synchronized void onResponse(Response response) throws IOException {
39-
Buffer buffer = new Buffer();
40-
ResponseBody body = response.body();
41-
body.source().readAll(buffer);
42-
43-
responses.add(new RecordedResponse(response.request(), response, null, buffer.readUtf8(), null));
38+
String body = response.body().string();
39+
responses.add(new RecordedResponse(response.request(), response, null, body, null));
4440
notifyAll();
4541
}
4642

‎okhttp-urlconnection/src/test/java/com/squareup/okhttp/OkUrlFactoryTest.java

+14-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.squareup.okhttp;
22

33
import com.squareup.okhttp.internal.Platform;
4-
import com.squareup.okhttp.internal.io.FileSystem;
54
import com.squareup.okhttp.internal.io.InMemoryFileSystem;
65
import com.squareup.okhttp.mockwebserver.MockResponse;
76
import com.squareup.okhttp.mockwebserver.MockWebServer;
@@ -14,6 +13,8 @@
1413
import java.util.Locale;
1514
import java.util.TimeZone;
1615
import java.util.concurrent.TimeUnit;
16+
import okio.BufferedSource;
17+
import org.junit.After;
1718
import org.junit.Before;
1819
import org.junit.Rule;
1920
import org.junit.Test;
@@ -26,16 +27,22 @@
2627

2728
public class OkUrlFactoryTest {
2829
@Rule public MockWebServer server = new MockWebServer();
30+
@Rule public InMemoryFileSystem fileSystem = new InMemoryFileSystem();
2931

30-
private FileSystem fileSystem = new InMemoryFileSystem();
3132
private OkUrlFactory factory;
33+
private Cache cache;
3234

3335
@Before public void setUp() throws IOException {
3436
OkHttpClient client = new OkHttpClient();
35-
client.setCache(new Cache(new File("/cache/"), 10 * 1024 * 1024, fileSystem));
37+
cache = new Cache(new File("/cache/"), 10 * 1024 * 1024, fileSystem);
38+
client.setCache(cache);
3639
factory = new OkUrlFactory(client);
3740
}
3841

42+
@After public void tearDown() throws IOException {
43+
cache.delete();
44+
}
45+
3946
/**
4047
* Response code 407 should only come from proxy servers. Android's client
4148
* throws if it is sent by an origin server.
@@ -64,6 +71,7 @@ public class OkUrlFactoryTest {
6471

6572
HttpURLConnection connection = factory.open(server.getUrl("/"));
6673
assertResponseHeader(connection, "NETWORK 404");
74+
connection.getErrorStream().close();
6775
}
6876

6977
@Test public void conditionalCacheHitResponseSourceHeaders() throws Exception {
@@ -140,7 +148,9 @@ public void setInstanceFollowRedirectsFalse() throws Exception {
140148
}
141149

142150
private void assertResponseBody(HttpURLConnection connection, String expected) throws Exception {
143-
String actual = buffer(source(connection.getInputStream())).readString(US_ASCII);
151+
BufferedSource source = buffer(source(connection.getInputStream()));
152+
String actual = source.readString(US_ASCII);
153+
source.close();
144154
assertEquals(expected, actual);
145155
}
146156

‎okhttp-urlconnection/src/test/java/com/squareup/okhttp/UrlConnectionCacheTest.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.squareup.okhttp.internal.Internal;
2020
import com.squareup.okhttp.internal.SslContextBuilder;
2121
import com.squareup.okhttp.internal.Util;
22-
import com.squareup.okhttp.internal.io.FileSystem;
2322
import com.squareup.okhttp.internal.io.InMemoryFileSystem;
2423
import com.squareup.okhttp.mockwebserver.MockResponse;
2524
import com.squareup.okhttp.mockwebserver.MockWebServer;
@@ -81,9 +80,9 @@ public final class UrlConnectionCacheTest {
8180

8281
@Rule public MockWebServer server = new MockWebServer();
8382
@Rule public MockWebServer server2 = new MockWebServer();
83+
@Rule public InMemoryFileSystem fileSystem = new InMemoryFileSystem();
8484

8585
private final SSLContext sslContext = SslContextBuilder.localhost();
86-
private final FileSystem fileSystem = new InMemoryFileSystem();
8786
private final OkUrlFactory client = new OkUrlFactory(new OkHttpClient());
8887
private Cache cache;
8988
private final CookieManager cookieManager = new CookieManager();
@@ -98,6 +97,7 @@ public final class UrlConnectionCacheTest {
9897
@After public void tearDown() throws Exception {
9998
ResponseCache.setDefault(null);
10099
CookieHandler.setDefault(null);
100+
cache.delete();
101101
}
102102

103103
@Test public void responseCacheAccessWithOkHttpMember() throws IOException {
@@ -1586,6 +1586,7 @@ public void assertCookies(URL url, String... expectedCookies) throws Exception {
15861586

15871587
HttpURLConnection connection = client.open(server.getUrl("/"));
15881588
assertEquals("A", connection.getHeaderField(""));
1589+
assertEquals("body", readAscii(connection));
15891590
}
15901591

15911592
/**

0 commit comments

Comments
 (0)