1

I have an image "manager" that downloads images. Previously I used the Picasso library for this as follows

class DownloadImage implements Runnable {

    String url;
    Context context;

    public DownloadImage(String url, Context context) {
        this.url = url;
        this.context = context;
    }

    @Override
    public void run() {
        try {
            String hash = Utilities.getSha1Hex(url);
            FileOutputStream fos = context.openFileOutput(hash, Context.MODE_PRIVATE);
            Bitmap bitmap = Picasso.with(context)
                    .load(url)
                    .resize(1024, 0) // Height 0 to ensure the image is scaled with respect to width - http://stackoverflow.com/a/26782046/1360853
                    .onlyScaleDown()
                    .memoryPolicy(MemoryPolicy.NO_CACHE)
                    .get();

            // Writing the bitmap to the output stream
            bitmap.compress(Bitmap.CompressFormat.JPEG, 80, fos);
            fos.close();
            bitmap.recycle();
        } catch (IOException e) {
            Timber.e(e, "For url %s", url);
        } catch (OutOfMemoryError e) {
            Timber.e(e, "out of memory for url %s", url);
        }
    }
}

But this creates a Bitmap object which not only consumes a lot of memory, it is also considerably slower and unnecessary.

I have modified this Runnable to use okhttp3 instead:

class DownloadImage implements Runnable {

    String url;
    Context context;

    public DownloadImage(String url, Context context) {
        this.url = url;
        this.context = context;
    }

    @Override
    public void run() {
        try {
            String hash = Utilities.getSha1Hex(url);
            final FileOutputStream fos = context.openFileOutput(hash, Context.MODE_PRIVATE);
            Request request = new Request.Builder().url(url).build();
            okHttpClient.newCall(request).enqueue(new Callback() {
                @Override
                public void onFailure(Call call, IOException e) {
                    try {
                        fos.close();
                    } catch (IOException e1) {
                        e1.printStackTrace();
                    }
                }

                @Override
                public void onResponse(Call call, Response response) throws IOException {
                    Sink sink = null;
                    BufferedSource source = null;
                    try {
                        source = response.body().source();
                        sink = Okio.sink(fos);
                        source.readAll(sink);
                    } catch (Exception e) {
                        Timber.e(e, "Downloading an image went wrong");
                    } finally {
                        if (source != null) {
                            source.close();
                        }
                        if (sink != null) {
                            sink.close();
                        }
                        fos.close();
                        okHttpClient.connectionPool().evictAll(); // For testing
                    }
                }
            });
        } catch (IOException e) {
            Timber.e(e, "For url %s", url);
        }
    }
}

While this approach is A LOT faster than the previous, for a large number of images I get A/libc: FORTIFY_SOURCE: FD_SET: file descriptor >= FD_SETSIZE. Calling abort(). followed by a microdump, which means I have too many file descriptors open.

For testing sake I have added the okHttpClient.connectionPool().evictAll(); // For testing line, but that didn't work. I also tried setting builder.connectionPool(new ConnectionPool(4, 500, TimeUnit.MILLISECONDS)); when building the okHttpClient, but that did nothing either. I am also aware of https://github.com/square/okhttp/issues/2636

I seem to close all streams/sinks/sources, so what is going on here?

The runnables are added to a ThreadPoolExecutor using its execute function, which is created as follows:

// Sets the amount of time an idle thread waits before terminating
private static final int KEEP_ALIVE_TIME = 500;
// Sets the Time Unit to milliseconds
private static final TimeUnit KEEP_ALIVE_TIME_UNIT = TimeUnit.MILLISECONDS;
private static int NUMBER_OF_CORES = Runtime.getRuntime().availableProcessors();

// A queue of Runnables
private final BlockingQueue<Runnable> mDecodeWorkQueue;
private OkHttpClient okHttpClient;

ThreadPoolExecutor mDecodeThreadPool;

public ImageManager() {
    // Instantiates the queue of Runnables as a LinkedBlockingQueue
    mDecodeWorkQueue = new LinkedBlockingQueue<Runnable>();

    // Creates a thread pool manager
    mDecodeThreadPool = new ThreadPoolExecutor(
            NUMBER_OF_CORES,       // Initial pool size
            NUMBER_OF_CORES,       // Max pool size
            KEEP_ALIVE_TIME,
            KEEP_ALIVE_TIME_UNIT,
            mDecodeWorkQueue);

}
Gooey
  • 4,740
  • 10
  • 42
  • 76
  • We ran into this when doing multi-threaded networking and running into lots of `400` responses (intentional) during this. What fixed it was ensuring that the `response` was being closed if it encountered an unsuccessful request, by doing `response.close()`. This must free up the socket immediately as opposed to waiting for some kind of timeout that can be too long and end up using all the file descriptors up before they have a chance to be let go automatically. Read more here: https://square.github.io/okhttp/4.x/okhttp/okhttp3/-response-body/ – Joshua Pinter Aug 30 '21 at 22:32

1 Answers1

2

Solved it by creating and using the FileOutputStream in the OnResponse body, so that it's not open while the request is being done.

Gooey
  • 4,740
  • 10
  • 42
  • 76