Improper creation of OkHttpClient in Websocket.java probably causing Out Of Memory exceptions

[shobhitpuri]

Issue:
I have been having issue with 100's of Out of Memory exceptions in my Android app which is using socket.io-client-java library since couple of months and I've been trying a lot to figure out. The library uses engine.io library.

Hypothesis:
After doing some exploration, this is the hypothesis I have. Please feel free to suggest your thoughts or a fix. In Websocket.java file, each time doOpen() is being called, a new instance of OkHttpClient is being initialized as follows:

public void doOpen() {
     // ..... 
     OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder()
                // turn off timeouts (github.com/socketio/engine.io-client-java/issues/32)
                .connectTimeout(0, TimeUnit.MILLISECONDS)
                .readTimeout(0, TimeUnit.MILLISECONDS)
                .writeTimeout(0, TimeUnit.MILLISECONDS);
    // ...
    final OkHttpClient client = clientBuilder.build();
    // ...
}

Based on a comment by swankjesse at Square, on an issue opened on okhttp regarding the OOM exceptions, he suggested to share the instance of OkHttpClient. Otherwise each time when we are creating a new instance of OkHttpClient, it will hold its own connection pool and thread pool, which is what seems to be happening when calling doOpen() in Websocket.java. An app which closes connection and opens connection multiple times in its workflow is having lot of OOM exceptions (stacktrace same as on square/okhttp#2846). When a mobile app goes in background, one would want to close connection to prevent using resources and then re-open connection when app is visible.

From the okhttp docs:

OkHttp performs best when you create a single OkHttpClient instance and reuse it for all of your HTTP calls. This is because each client holds its own connection pool and thread pools. Reusing connections and threads reduces latency and saves memory. Conversely, creating a client for each request wastes resources on idle pools.

Solution seems to be fixing it as mentioned in the docs. Feel free to comment if the hypothesis seems wrong.

Thanks

References:

1. Crashes on OutOfMemoryError, while there are 257 "OkHttp ConnectionPool" threads square/okhttp#2846
2. https://square.github.io/okhttp/3.x/okhttp/okhttp3/OkHttpClient.html
3. Socket.io Java client use this library and it is causing OOM's in them. OutOfMemoryError: pthread_create socket.io-client-java#315

[b95505017]

Maybe we should let develop inject their OkHttpClient so an application could use only one client instance.

[Jashepp]

I am having this issue, in the case of my Socket.IO server being offline and the java client trying to connect (or reconnect), creating new threads that don't close, forever. So I have attempted to make a fix in my own fork, which seems to be working for my application.
With my changes, I am simply passing shareOkHttpClient as true to the socket options.
Feel free to use the same changes for the fix. https://github.com/Unchosen/engine.io-client-java/commit/51d41d64e98581efb90a6d30ddefaa3063552914

[shobhitpuri]

@unchosen That's great news! I see that you forked the original repo. Can you please make a pull request to the original repo as well?

[shobhitpuri]

@b95505017 Can you please look into the pull request #82 by @unchosen when you get time? Hopefully it solves this issue. Thanks so much.

[b95505017]

@shobhitpuri Ok I'll review it ASAP.

[nkzawa]

The fix was released as 0.9.0, thanks to @b95505017. Let us know when the problem still happens.

[KevinLiu707]

[OOM still happended] (socketio/socket.io-client-java#786) @nkzawa @shobhitpuri . The real reason is the threadpool closed when the task is done in EventThread class . But the same code is still in 0.9.0 nor later version.