Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crashes on OutOfMemoryError, while there are 257 "OkHttp ConnectionPool" threads #2846

Closed
marmor7 opened this issue Sep 11, 2016 · 5 comments

Comments

@marmor7
Copy link

marmor7 commented Sep 11, 2016

We've recently migrated our app to OkHttp, and we've now getting several crashes for out-of-memory related errors.
Looking at the crashes we see hundreds of threads running at:
okhttp3.ConnectionPool$1.run (ConnectionPool.java:66)

The stack of the crash itself is:

Fatal Exception: java.lang.OutOfMemoryError: Could not allocate JNI Env
at java.lang.Thread.nativeCreate(Thread.java)
at java.lang.Thread.start(Thread.java:1063)
at java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:921)
at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1339)
at okhttp3.ConnectionPool.put(ConnectionPool.java:135)
at okhttp3.OkHttpClient$1.put(OkHttpClient.java:149)
at okhttp3.internal.connection.StreamAllocation.findConnection(StreamAllocation.java:188)
at okhttp3.internal.connection.StreamAllocation.findHealthyConnection(StreamAllocation.java:129)
at okhttp3.internal.connection.StreamAllocation.newStream(StreamAllocation.java:98)
at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:42)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:109)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:124)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:170)
at okhttp3.RealCall.execute(RealCall.java:60)
at MyActivity.myFunction(MyActivity.java:853)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
at java.lang.Thread.run(Thread.java:818)

Shouldn't there be a limit on how many threads can OkHttp create?

@swankjesse
Copy link
Collaborator

You want to read this advice:
https://square.github.io/okhttp/3.x/okhttp/okhttp3/OkHttpClient.html

@marmor7
Copy link
Author

marmor7 commented Sep 11, 2016

Thanks for the quick reply.
We were under the impression that builder() will return an existing instance, we'll get that fixed, thanks! 👍

@shobhitpuri
Copy link

shobhitpuri commented Jan 24, 2017

@marmor7 Do you mean to say if we use builder() like following inside a method and call that method multiple times, each time a new instance of the client will be created and each instance will holds its own connection pool and thread pools?

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();
    // ...
}

Reference: Websocket.java for engine.io-client-java project.

@marmor7
Copy link
Author

marmor7 commented Jan 25, 2017

@shobhitpuri yes, that's exactly what we did, and had ton of memory issues.

From the 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.

Instead, we now have a single OkHttpClient that we build once, and is used by all methods.

@Arun-search
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants