Skip to content

Commit 6c996ea

Browse files
danikuladanikula
committed
🐛 don't close source after processing partial request without cache #43
prevent invalid calling source.close() in different threads to avoid crashes on Lollipop (#37, #29, #63, #66)
1 parent 582832f commit 6c996ea

File tree

5 files changed

+121
-27
lines changed

5 files changed

+121
-27
lines changed

‎files/space.jpg‎

1.03 MB
Loading

‎library/src/main/java/com/danikula/videocache/HttpProxyCache.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,18 @@ private void responseWithCache(OutputStream out, long offset) throws ProxyCacheE
8383
}
8484

8585
private void responseWithoutCache(OutputStream out, long offset) throws ProxyCacheException, IOException {
86+
HttpUrlSource newSourceNoCache = new HttpUrlSource(this.source);
8687
try {
87-
HttpUrlSource source = new HttpUrlSource(this.source);
88-
source.open((int) offset);
88+
newSourceNoCache.open((int) offset);
8989
byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
9090
int readBytes;
91-
while ((readBytes = source.read(buffer)) != -1) {
91+
while ((readBytes = newSourceNoCache.read(buffer)) != -1) {
9292
out.write(buffer, 0, readBytes);
9393
offset += readBytes;
9494
}
9595
out.flush();
9696
} finally {
97-
source.close();
97+
newSourceNoCache.close();
9898
}
9999
}
100100

‎library/src/main/java/com/danikula/videocache/HttpUrlSource.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,11 @@ public void close() throws ProxyCacheException {
7878
if (connection != null) {
7979
try {
8080
connection.disconnect();
81-
} catch (NullPointerException e) {
82-
// https://github.com/danikula/AndroidVideoCache/issues/32
83-
// https://github.com/danikula/AndroidVideoCache/issues/29
84-
throw new ProxyCacheException("Error disconnecting HttpUrlConnection", e);
81+
} catch (NullPointerException | IllegalArgumentException e) {
82+
String message = "Wait... but why? WTF!? " +
83+
"Really shouldn't happen any more after fixing https://github.com/danikula/AndroidVideoCache/issues/43. " +
84+
"If you read it on your device log, please, notify me [email protected] or create issue here https://github.com/danikula/AndroidVideoCache/issues.";
85+
throw new RuntimeException(message, e);
8586
}
8687
}
8788
}

‎test/src/main/assets/space.jpg‎

1.03 MB
Loading

‎test/src/test/java/com/danikula/videocache/HttpProxyCacheTest.java‎

Lines changed: 112 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,20 @@
1313

1414
import java.io.ByteArrayOutputStream;
1515
import java.io.File;
16+
import java.io.IOException;
1617
import java.net.Socket;
18+
import java.util.Random;
19+
import java.util.concurrent.Callable;
20+
import java.util.concurrent.CountDownLatch;
21+
import java.util.concurrent.ExecutorService;
22+
import java.util.concurrent.Executors;
23+
import java.util.concurrent.Future;
1724

25+
import static com.danikula.videocache.support.ProxyCacheTestUtils.ASSETS_DATA_BIG_NAME;
26+
import static com.danikula.videocache.support.ProxyCacheTestUtils.HTTP_DATA_BIG_URL;
27+
import static com.danikula.videocache.support.ProxyCacheTestUtils.HTTP_DATA_SIZE;
1828
import static com.danikula.videocache.support.ProxyCacheTestUtils.HTTP_DATA_URL;
29+
import static com.danikula.videocache.support.ProxyCacheTestUtils.loadAssetFile;
1930
import static com.danikula.videocache.support.ProxyCacheTestUtils.loadTestData;
2031
import static org.fest.assertions.api.Assertions.assertThat;
2132
import static org.mockito.Matchers.any;
@@ -37,37 +48,22 @@ public class HttpProxyCacheTest {
3748

3849
@Test
3950
public void testProcessRequestNoCache() throws Exception {
40-
HttpUrlSource source = new HttpUrlSource(HTTP_DATA_URL);
41-
FileCache cache = new FileCache(ProxyCacheTestUtils.newCacheFile());
42-
HttpProxyCache proxyCache = new HttpProxyCache(source, cache);
43-
GetRequest request = new GetRequest("GET /" + HTTP_DATA_URL + " HTTP/1.1");
44-
ByteArrayOutputStream out = new ByteArrayOutputStream();
45-
Socket socket = mock(Socket.class);
46-
when(socket.getOutputStream()).thenReturn(out);
47-
48-
proxyCache.processRequest(request, socket);
49-
Response response = new Response(out.toByteArray());
51+
Response response = processRequest(HTTP_DATA_URL, "GET /" + HTTP_DATA_URL + " HTTP/1.1");
5052

5153
assertThat(response.data).isEqualTo(loadTestData());
5254
assertThat(response.code).isEqualTo(200);
53-
assertThat(response.contentLength).isEqualTo(ProxyCacheTestUtils.HTTP_DATA_SIZE);
55+
assertThat(response.contentLength).isEqualTo(HTTP_DATA_SIZE);
5456
assertThat(response.contentType).isEqualTo("image/jpeg");
5557
}
5658

5759
@Test
5860
public void testProcessPartialRequestWithoutCache() throws Exception {
59-
HttpUrlSource source = new HttpUrlSource(HTTP_DATA_URL);
6061
FileCache fileCache = new FileCache(ProxyCacheTestUtils.newCacheFile());
6162
FileCache spyFileCache = Mockito.spy(fileCache);
6263
doThrow(new RuntimeException()).when(spyFileCache).read(any(byte[].class), anyLong(), anyInt());
63-
HttpProxyCache proxyCache = new HttpProxyCache(source, spyFileCache);
64-
GetRequest request = new GetRequest("GET /" + HTTP_DATA_URL + " HTTP/1.1\nRange: bytes=2000-");
65-
ByteArrayOutputStream out = new ByteArrayOutputStream();
66-
Socket socket = mock(Socket.class);
67-
when(socket.getOutputStream()).thenReturn(out);
6864

69-
proxyCache.processRequest(request, socket);
70-
Response response = new Response(out.toByteArray());
65+
String httpRequest = "GET /" + HTTP_DATA_URL + " HTTP/1.1\nRange: bytes=2000-";
66+
Response response = processRequest(HTTP_DATA_URL, httpRequest, spyFileCache);
7167

7268
byte[] fullData = loadTestData();
7369
byte[] partialData = new byte[fullData.length - 2000];
@@ -76,6 +72,73 @@ public void testProcessPartialRequestWithoutCache() throws Exception {
7672
assertThat(response.code).isEqualTo(206);
7773
}
7874

75+
@Test // https://github.com/danikula/AndroidVideoCache/issues/43
76+
public void testPreventClosingOriginalSourceForNewPartialRequestWithoutCache() throws Exception {
77+
HttpUrlSource source = new HttpUrlSource(HTTP_DATA_BIG_URL);
78+
FileCache fileCache = new FileCache(ProxyCacheTestUtils.newCacheFile());
79+
HttpProxyCache proxyCache = new HttpProxyCache(source, fileCache);
80+
ExecutorService executor = Executors.newFixedThreadPool(5);
81+
Future<Response> firstRequestFeature = processAsync(executor, proxyCache, "GET /" + HTTP_DATA_URL + " HTTP/1.1");
82+
Thread.sleep(100); // wait for first request started to process
83+
84+
int offset = 30000;
85+
String partialRequest = "GET /" + HTTP_DATA_URL + " HTTP/1.1\nRange: bytes=" + offset + "-";
86+
Future<Response> secondRequestFeature = processAsync(executor, proxyCache, partialRequest);
87+
88+
Response secondResponse = secondRequestFeature.get();
89+
Response firstResponse = firstRequestFeature.get();
90+
91+
byte[] responseData = loadAssetFile(ASSETS_DATA_BIG_NAME);
92+
assertThat(firstResponse.data).isEqualTo(responseData);
93+
94+
byte[] partialData = new byte[responseData.length - offset];
95+
System.arraycopy(responseData, offset, partialData, 0, partialData.length);
96+
assertThat(secondResponse.data).isEqualTo(partialData);
97+
}
98+
99+
@Test
100+
public void testProcessManyThreads() throws Exception {
101+
final String url = "https://raw.githubusercontent.com/danikula/AndroidVideoCache/master/files/space.jpg";
102+
HttpUrlSource source = new HttpUrlSource(url);
103+
FileCache fileCache = new FileCache(ProxyCacheTestUtils.newCacheFile());
104+
final HttpProxyCache proxyCache = new HttpProxyCache(source, fileCache);
105+
final byte[] loadedData = loadAssetFile("space.jpg");
106+
final Random random = new Random(System.currentTimeMillis());
107+
int concurrentRequests = 10;
108+
ExecutorService executor = Executors.newFixedThreadPool(concurrentRequests);
109+
Future[] results = new Future[concurrentRequests];
110+
int[] offsets = new int[concurrentRequests];
111+
final CountDownLatch finishLatch = new CountDownLatch(concurrentRequests);
112+
final CountDownLatch startLatch = new CountDownLatch(1);
113+
for (int i = 0; i < concurrentRequests; i++) {
114+
final int offset = random.nextInt(loadedData.length);
115+
offsets[i] = offset;
116+
results[i] = executor.submit(new Callable<Response>() {
117+
118+
@Override
119+
public Response call() throws Exception {
120+
try {
121+
startLatch.await();
122+
String partialRequest = "GET /" + url + " HTTP/1.1\nRange: bytes=" + offset + "-";
123+
return processRequest(proxyCache, partialRequest);
124+
} finally {
125+
finishLatch.countDown();
126+
}
127+
}
128+
});
129+
}
130+
startLatch.countDown();
131+
finishLatch.await();
132+
133+
for (int i = 0; i < results.length; i++) {
134+
Response response = (Response) results[i].get();
135+
int offset = offsets[i];
136+
byte[] partialData = new byte[loadedData.length - offset];
137+
System.arraycopy(loadedData, offset, partialData, 0, partialData.length);
138+
assertThat(response.data).isEqualTo(partialData);
139+
}
140+
}
141+
79142
@Test
80143
public void testLoadEmptyFile() throws Exception {
81144
String zeroSizeUrl = "https://raw.githubusercontent.com/danikula/AndroidVideoCache/master/files/empty.txt";
@@ -95,4 +158,34 @@ public void testLoadEmptyFile() throws Exception {
95158
Mockito.verify(listener).onCacheAvailable(Mockito.<File>any(), eq(zeroSizeUrl), eq(100));
96159
assertThat(response.data).isEmpty();
97160
}
161+
162+
private Response processRequest(String sourceUrl, String httpRequest) throws ProxyCacheException, IOException {
163+
FileCache fileCache = new FileCache(ProxyCacheTestUtils.newCacheFile());
164+
return processRequest(sourceUrl, httpRequest, fileCache);
165+
}
166+
167+
private Response processRequest(String sourceUrl, String httpRequest, FileCache fileCache) throws ProxyCacheException, IOException {
168+
HttpUrlSource source = new HttpUrlSource(sourceUrl);
169+
HttpProxyCache proxyCache = new HttpProxyCache(source, fileCache);
170+
return processRequest(proxyCache, httpRequest);
171+
}
172+
173+
private Response processRequest(HttpProxyCache proxyCache, String httpRequest) throws ProxyCacheException, IOException {
174+
GetRequest request = new GetRequest(httpRequest);
175+
ByteArrayOutputStream out = new ByteArrayOutputStream();
176+
Socket socket = mock(Socket.class);
177+
when(socket.getOutputStream()).thenReturn(out);
178+
proxyCache.processRequest(request, socket);
179+
return new Response(out.toByteArray());
180+
}
181+
182+
private Future<Response> processAsync(ExecutorService executor, final HttpProxyCache proxyCache, final String httpRequest) {
183+
return executor.submit(new Callable<Response>() {
184+
185+
@Override
186+
public Response call() throws Exception {
187+
return processRequest(proxyCache, httpRequest);
188+
}
189+
});
190+
}
98191
}

0 commit comments

Comments
 (0)