Handle and throw exceptions in read in downsampler.

Fixes #126

library/src/androidTest/java/com/bumptech/glide/util/ExceptionCatchingInputStreamTest.java

+package com.bumptech.glide.util;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.SocketTimeoutException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class ExceptionCatchingInputStreamTest {
+
+    private InputStream wrapped;
+    private ExceptionCatchingInputStream is;
+
+    @Before
+    public void setUp() throws Exception {
+        wrapped = mock(InputStream.class);
+        is = new ExceptionCatchingInputStream();
+        is.setInputStream(wrapped);
+    }
+
+    @After
+    public void tearDown() {
+        ExceptionCatchingInputStream.clearQueue();
+    }
+
+    @Test
+    public void testReturnsWrappedAvailable() throws IOException {
+        when(wrapped.available()).thenReturn(25);
+        assertEquals(25, is.available());
+    }
+
+    @Test
+    public void testCallsCloseOnWrapped() throws IOException {
+        is.close();
+        verify(wrapped).close();
+    }
+
+    @Test
+    public void testCallsMarkOnWrapped() {
+        int toMark = 50;
+        is.mark(toMark);
+        verify(wrapped).mark(eq(toMark));
+    }
+
+    @Test
+    public void testReturnsWrappedMarkSupported() {
+        when(wrapped.markSupported()).thenReturn(true);
+        assertTrue(is.markSupported());
+    }
+
+    @Test
+    public void testCallsReadByteArrayOnWrapped() throws IOException {
+        byte[] buffer = new byte[100];
+        when(wrapped.read(eq(buffer))).thenReturn(buffer.length);
+        assertEquals(buffer.length, is.read(buffer));
+    }
+
+    @Test
+    public void testCallsReadArrayWithOffsetAndCountOnWrapped() throws IOException {
+        int offset = 5;
+        int count = 100;
+        byte[] buffer = new byte[105];
+
+        when(wrapped.read(eq(buffer), eq(offset), eq(count))).thenReturn(count);
+        assertEquals(count, is.read(buffer, offset, count));
+    }
+
+    @Test
+    public void testCallsReadOnWrapped() throws IOException {
+        when(wrapped.read()).thenReturn(1);
+        assertEquals(1, is.read());
+    }
+
+    @Test
+    public void testCallsResetOnWrapped() throws IOException {
+        is.reset();
+        verify(wrapped).reset();
+    }
+
+    @Test
+    public void testCallsSkipOnWrapped() throws IOException {
+        long toSkip = 67;
+        long expected = 55;
+        when(wrapped.skip(eq(toSkip))).thenReturn(expected);
+        assertEquals(expected, is.skip(toSkip));
+    }
+
+    @Test
+    public void testCatchesExceptionOnRead() throws IOException {
+        IOException expected = new SocketTimeoutException();
+        when(wrapped.read()).thenThrow(expected);
+        int read = is.read();
+
+        assertEquals(-1, read);
+        assertEquals(expected, is.getException());
+    }
+
+    @Test
+    public void testCatchesExceptionOnReadBuffer() throws IOException {
+        IOException exception = new SocketTimeoutException();
+        when(wrapped.read(any(byte[].class))).thenThrow(exception);
+
+        int read = is.read(new byte[0]);
+        assertEquals(-1, read);
+        assertEquals(exception, is.getException());
+    }
+
+    @Test
+    public void testCatchesExceptionOnReadBufferWithOffsetAndCount() throws IOException {
+        IOException exception = new SocketTimeoutException();
+        when(wrapped.read(any(byte[].class), anyInt(), anyInt())).thenThrow(exception);
+
+        int read = is.read(new byte[0], 10, 100);
+        assertEquals(-1, read);
+        assertEquals(exception, is.getException());
+    }
+
+    @Test
+    public void testCatchesExceptionOnSkip() throws IOException {
+        IOException exception = new SocketTimeoutException();
+        when(wrapped.skip(anyLong())).thenThrow(exception);
+
+        long skipped = is.skip(100);
+        assertEquals(0, skipped);
+        assertEquals(exception, is.getException());
+    }
+
+    @Test
+    public void testExceptionIsNotSetInitially() {
+        assertNull(is.getException());
+    }
+
+    @SuppressWarnings("ResultOfMethodCallIgnored")
+    @Test
+    public void testResetsExceptionToNullOnRelease() throws IOException {
+        IOException exception = new SocketTimeoutException();
+        when(wrapped.read()).thenThrow(exception);
+        is.read();
+        is.release();
+        assertNull(is.getException());
+    }
+
+    @Test
+    public void testCanReleaseAnObtainFromPool() {
+        is.release();
+        InputStream fromPool = ExceptionCatchingInputStream.obtain(wrapped);
+        assertEquals(is, fromPool);
+    }
+
+    @Test
+    public void testCanObtainNewStreamFromPool() throws IOException {
+        InputStream fromPool = ExceptionCatchingInputStream.obtain(wrapped);
+        when(wrapped.read()).thenReturn(1);
+        int read = fromPool.read();
+        assertEquals(1, read);
+    }
+}
\ No newline at end of file

library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java

@@ -9,6 +9,7 @@ import android.util.Log;
 import com.bumptech.glide.load.DecodeFormat;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
 import com.bumptech.glide.util.ByteArrayPool;
+import com.bumptech.glide.util.ExceptionCatchingInputStream;
 import com.bumptech.glide.util.Util;
 
 import java.io.IOException;
@@ -140,19 +141,21 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
         final byte[] bytesForOptions = byteArrayPool.getBytes();
         final byte[] bytesForStream = byteArrayPool.getBytes();
         final BitmapFactory.Options options = getDefaultOptions();
-        final RecyclableBufferedInputStream bis = new RecyclableBufferedInputStream(is, bytesForStream);
+        // TODO(#126): when the framework handles exceptions better, consider removing.
+        final ExceptionCatchingInputStream stream =
+                ExceptionCatchingInputStream.obtain(new RecyclableBufferedInputStream(is, bytesForStream));
         try {
-            bis.mark(MARK_POSITION);
+            stream.mark(MARK_POSITION);
             int orientation = 0;
             try {
-                orientation = new ImageHeaderParser(bis).getOrientation();
+                orientation = new ImageHeaderParser(stream).getOrientation();
             } catch (IOException e) {
                 if (Log.isLoggable(TAG, Log.WARN)) {
                     Log.w(TAG, "Cannot determine the image orientation from header", e);
                 }
             } finally {
                 try {
-                    bis.reset();
+                    stream.reset();
                 } catch (IOException e) {
                     if (Log.isLoggable(TAG, Log.WARN)) {
                         Log.w(TAG, "Cannot reset the input stream", e);
@@ -162,7 +165,7 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
 
             options.inTempStorage = bytesForOptions;
 
-            final int[] inDimens = getDimensions(bis, options);
+            final int[] inDimens = getDimensions(stream, options);
             final int inWidth = inDimens[0];
             final int inHeight = inDimens[1];
 
@@ -177,7 +180,15 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
             }
 
             final Bitmap downsampled =
-                    downsampleWithSize(bis, options, pool, inWidth, inHeight, sampleSize, decodeFormat);
+                    downsampleWithSize(stream, options, pool, inWidth, inHeight, sampleSize, decodeFormat);
+
+            // BitmapDecoder swallows exceptions during decodes and in some cases when inBitmap is non null, may catch
+            // and log a stack trace but still return a non null bitmap. To avoid displaying partially decoded bitmaps,
+            // we catch exceptions reading from the stream in our ExceptionCatchingInputStream and throw them here.
+            final Exception streamException = stream.getException();
+            if (streamException != null) {
+                throw new RuntimeException(streamException);
+            }
 
             Bitmap rotated = null;
             if (downsampled != null) {
@@ -192,33 +203,34 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
         } finally {
             byteArrayPool.releaseBytes(bytesForOptions);
             byteArrayPool.releaseBytes(bytesForStream);
+            stream.release();
             releaseOptions(options);
         }
     }
 
-    protected Bitmap downsampleWithSize(RecyclableBufferedInputStream bis, BitmapFactory.Options options,
+    protected Bitmap downsampleWithSize(InputStream is, BitmapFactory.Options options,
             BitmapPool pool, int inWidth, int inHeight, int sampleSize, DecodeFormat decodeFormat) {
         // Prior to KitKat, the inBitmap size must exactly match the size of the bitmap we're decoding.
-        Bitmap.Config config = getConfig(bis, decodeFormat);
+        Bitmap.Config config = getConfig(is, decodeFormat);
         options.inSampleSize = sampleSize;
         options.inPreferredConfig = config;
         if (options.inSampleSize == 1 || Build.VERSION_CODES.KITKAT <= Build.VERSION.SDK_INT) {
-            if (shouldUsePool(bis)) {
+            if (shouldUsePool(is)) {
                 setInBitmap(options, pool.get(inWidth, inHeight, config));
             }
         }
-        return decodeStream(bis, options);
+        return decodeStream(is, options);
     }
 
-    private static boolean shouldUsePool(RecyclableBufferedInputStream bis) {
+    private static boolean shouldUsePool(InputStream is) {
         // On KitKat+, any bitmap can be used to decode any other bitmap.
         if (Build.VERSION_CODES.KITKAT <= Build.VERSION.SDK_INT) {
             return true;
         }
 
-        bis.mark(1024);
+        is.mark(1024);
         try {
-            final ImageHeaderParser.ImageType type = new ImageHeaderParser(bis).getType();
+            final ImageHeaderParser.ImageType type = new ImageHeaderParser(is).getType();
             // cannot reuse bitmaps when decoding images that are not PNG or JPG.
             // look at : https://groups.google.com/forum/#!msg/android-developers/Mp0MFVFi1Fo/e8ZQ9FGdWdEJ
             return TYPES_THAT_USE_POOL.contains(type);
@@ -228,7 +240,7 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
             }
         } finally {
             try {
-                bis.reset();
+                is.reset();
             } catch (IOException e) {
                 if (Log.isLoggable(TAG, Log.WARN)) {
                     Log.w(TAG, "Cannot reset the input stream", e);
@@ -238,23 +250,23 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
         return false;
     }
 
-    private static Bitmap.Config getConfig(RecyclableBufferedInputStream bis, DecodeFormat format) {
+    private static Bitmap.Config getConfig(InputStream is, DecodeFormat format) {
         if (format == DecodeFormat.ALWAYS_ARGB_8888) {
             return Bitmap.Config.ARGB_8888;
         }
 
         boolean hasAlpha = false;
         // We probably only need 25, but this is safer (particularly since the buffer size is > 1024).
-        bis.mark(1024);
+        is.mark(1024);
         try {
-            hasAlpha = new ImageHeaderParser(bis).hasAlpha();
+            hasAlpha = new ImageHeaderParser(is).hasAlpha();
         } catch (IOException e) {
             if (Log.isLoggable(TAG, Log.WARN)) {
                 Log.w(TAG, "Cannot determine whether the image has alpha or not from header for format " + format, e);
             }
         } finally {
             try {
-                bis.reset();
+                is.reset();
             } catch (IOException e) {
                 if (Log.isLoggable(TAG, Log.WARN)) {
                     Log.w(TAG, "Cannot reset the input stream", e);
@@ -283,36 +295,35 @@ public abstract class Downsampler implements BitmapDecoder<InputStream> {
     /**
      * A method for getting the dimensions of an image from the given InputStream.
      *
-     * @param bis The InputStream representing the image.
+     * @param is The InputStream representing the image.
      * @param options The options to pass to
      *          {@link BitmapFactory#decodeStream(java.io.InputStream, android.graphics.Rect,
      *              android.graphics.BitmapFactory.Options)}.
      * @return an array containing the dimensions of the image in the form {width, height}.
      */
-    public int[] getDimensions(RecyclableBufferedInputStream bis, BitmapFactory.Options options) {
+    public int[] getDimensions(InputStream is, BitmapFactory.Options options) {
         options.inJustDecodeBounds = true;
-        decodeStream(bis, options);
+        decodeStream(is, options);
         options.inJustDecodeBounds = false;
         return new int[] { options.outWidth, options.outHeight };
     }
 
 
-    private static Bitmap decodeStream(RecyclableBufferedInputStream bis, BitmapFactory.Options options) {
+    private static Bitmap decodeStream(InputStream is, BitmapFactory.Options options) {
          if (options.inJustDecodeBounds) {
              // This is large, but jpeg headers are not size bounded so we need something large enough to minimize
              // the possibility of not being able to fit enough of the header in the buffer to get the image size so
              // that we don't fail to load images. The BufferedInputStream will create a new buffer of 2x the
              // original size each time we use up the buffer space without passing the mark so this is a maximum
              // bound on the buffer size, not a default. Most of the time we won't go past our pre-allocated 16kb.
-             bis.mark(MARK_POSITION);
+             is.mark(MARK_POSITION);
          }
 
-        final Bitmap result = BitmapFactory.decodeStream(bis, null, options);
+        final Bitmap result = BitmapFactory.decodeStream(is, null, options);
 
         try {
             if (options.inJustDecodeBounds) {
-                bis.reset();
-                bis.clearMark();
+                is.reset();
             }
         } catch (IOException e) {
             if (Log.isLoggable(TAG, Log.ERROR)) {

library/src/main/java/com/bumptech/glide/load/resource/bitmap/RecyclableBufferedInputStream.java

@@ -187,11 +187,6 @@ public class RecyclableBufferedInputStream extends FilterInputStream {
         markpos = pos;
     }
 
-    public synchronized void clearMark() {
-        markpos = -1;
-        marklimit = 0;
-    }
-
     /**
      * Indicates whether {@code BufferedInputStream} supports the {@link #mark(int)}
      * and {@link #reset()} methods.

library/src/main/java/com/bumptech/glide/util/ExceptionCatchingInputStream.java

+package com.bumptech.glide.util;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Queue;
+
+/**
+ * An {@link java.io.InputStream} that catches {@link java.io.IOException}s during read and skip calls and stores them
+ * so they can later be handled or thrown. This class is a workaround for a framework issue where exceptions during
+ * reads while decoding bitmaps in {@link android.graphics.BitmapFactory} can return partially decoded bitmaps.
+ *
+ * See https://github.com/bumptech/glide/issues/126.
+ */
+public class ExceptionCatchingInputStream extends InputStream {
+
+    private static final Queue<ExceptionCatchingInputStream> QUEUE = Util.createQueue(0);
+
+    public static ExceptionCatchingInputStream obtain(InputStream toWrap) {
+        ExceptionCatchingInputStream result;
+        synchronized (QUEUE) {
+            result = QUEUE.poll();
+        }
+        if (result == null) {
+            result = new ExceptionCatchingInputStream();
+        }
+        result.setInputStream(toWrap);
+        return result;
+    }
+
+    // Exposed for testing.
+    static void clearQueue() {
+
+    }
+
+    private InputStream wrapped;
+    private IOException exception;
+
+    ExceptionCatchingInputStream() {
+        // Do nothing.
+    }
+
+    void setInputStream(InputStream toWrap) {
+        wrapped = toWrap;
+    }
+
+    @Override
+    public int available() throws IOException {
+        return wrapped.available();
+    }
+
+    @Override
+    public void close() throws IOException {
+        wrapped.close();
+    }
+
+    @Override
+    public void mark(int readlimit) {
+        wrapped.mark(readlimit);
+    }
+
+    @Override
+    public boolean markSupported() {
+        return wrapped.markSupported();
+    }
+
+    @Override
+    public int read(byte[] buffer) throws IOException {
+        int read;
+        try {
+            read = wrapped.read(buffer);
+        } catch (IOException e) {
+            exception = e;
+            read = -1;
+        }
+        return read;
+    }
+
+    @Override
+    public int read(byte[] buffer, int byteOffset, int byteCount) throws IOException {
+        int read;
+        try {
+            read = wrapped.read(buffer, byteOffset, byteCount);
+        } catch (IOException e) {
+            exception = e;
+            read = -1;
+        }
+        return read;
+    }
+
+    @Override
+    public synchronized void reset() throws IOException {
+        wrapped.reset();
+    }
+
+    @Override
+    public long skip(long byteCount) throws IOException {
+        long skipped;
+        try {
+            skipped = wrapped.skip(byteCount);
+        } catch (IOException e) {
+            exception = e;
+            skipped = 0;
+        }
+        return skipped;
+    }
+
+    @Override
+    public int read() throws IOException {
+        int result;
+        try {
+            result = wrapped.read();
+        } catch (IOException e) {
+            exception = e;
+            result = -1;
+        }
+        return result;
+    }
+
+    public IOException getException() {
+        return exception;
+    }
+
+    public void release() {
+        exception = null;
+        wrapped = null;
+        synchronized (QUEUE) {
+            QUEUE.offer(this);
+        }
+    }
+}