Okhttp: How to avoid additional thread creation on synchronous request execution?

[koral--]

I want to send uncaught exception reports via HTTP POST. However recipe Posting a String from wiki cannot be used for that because InternalError is thrown on ART (stacktrace from version 2.1.0 of okhttp):

java.lang.InternalError: Thread starting during runtime shutdown
            at java.lang.Thread.nativeCreate(Native Method)
            at java.lang.Thread.start(Thread.java:1042)
            at java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:920)
            at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1336)
            at com.squareup.okhttp.ConnectionPool.get(ConnectionPool.java:206)
            at com.squareup.okhttp.internal.http.RouteSelector.nextUnconnected(RouteSelector.java:145)
            at com.squareup.okhttp.internal.http.RouteSelector.next(RouteSelector.java:133)
            at com.squareup.okhttp.internal.http.HttpEngine.connect(HttpEngine.java:314)
            at com.squareup.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:237)
            at com.squareup.okhttp.Call.getResponse(Call.java:233)
            at com.squareup.okhttp.Call.execute(Call.java:84)

I see that ConnectionPool is using ThreadPoolExecutor to perform cleanup and class is final so cannot be subclassed. Shouldn't there be option to do cleanup in caller thread?

[swankjesse]

Wait for the HTTP call to complete before exiting the runtime?

[swankjesse]

(Not possible in general, especially with SPDY.)

[koral--]

Did you understand problem correctly?
HTTP call is not causing the problem. Request itself can be performed without problem during shutdown. Problem is that new therad cannot be created.
This is the real cause: executorService.execute(connectionsCleanupRunnable);.
Why it is not possible to fix this? If mentioned call will be conditionally replaced by connectionsCleanupRunnable.run() wouldn't that work?

[swankjesse]

Yeah, I understand the problem. You shouldn't trigger a process shutdown until after you've done your reporting HTTP request. We cannot change OkHttp to not create threads when it executes requests; creating threads is a thing it needs to do to do its job.

[koral--]

I'm not triggering process shutdown. It has been already triggered by uncaught RuntimeException. I want to report that exception via request. It is a part of ReportSender in ACRA.
What about making ConnectionPool non-final?

[swankjesse]

The connection pool is just the first of many threads you'll face, and the other ones can't be killed as easily. For example, SPDY requires a background thread because it is asynchronous.

You need to fix your uncaught exception handler to not initiate shutdown.

[kaosko]

I have the same problem as @koral--, and like him I'm not initiating a shutdown. It's the ART. An uncaught exception is thrown and ACRA was able to send the report just fine under Dalvik but the same doesn't work under ART.

[swankjesse]

@kaosko where's the code that triggers VM shutdown?

[kaosko]

@swankjesse - what do you mean? The main thread exits due to an uncaught exception. At that point, ACRA tries to send a report, spawning a thread but ART will forcefully interrupt that.

[swankjesse]

You can set your own UncaughtExceptionHandler that doesn't trigger a VM exit.

[kaosko]

ACRA has already set an UncaughtExceptionHandler. ART terminates the VM while ACRA is trying to send an exception report because Thread.nativeCreate is not allowed during shutdown. If you are trying to say it's ACRA's problem that's fine, but I don't think it's possible to set another wrapper UncaughtExceptionHandler while retaining ACRA's behavior.

[swankjesse]

I think you should report this to ACRA. If you want to make HTTP requests in response to application crashes, we need to create threads.

[koral--]

It is possible to set your own UncaughtExceptionHandler and retain ACRA behavior. But It must be done before calling ACRA.init().
ACRA's developers won't probably do anything about this issue because it occurs only when custom ReportSender is used (so user should be responsible for it perhaps). Default ACRA's ReportSender is now unaffected (in 5.0.0-SNAPSHOT version, 4.50 is still affected) see https://github.com/ACRA/acra/issues/136.

ACRA stores the original (system default) uncaught exception handler and register itself (ErrorReporter namely) as new one. Uncaught exception flow is as follows (from https://github.com/ACRA/acra/blob/master/src/main/java/org/acra/ErrorReporter.java#L398):

1. uncaughtExeption() of ErrorReporter is called (in thread which exception has occurred in)
2. sender thread is created and started (shutdown has not been yet initiated so it is OK)
3. uncaughtExeption() of saved, default uncaught exception handler is called which initiates shutdown
4. run() of sender thread is called - this is place when HTTP requests are sent. Note that this method is called also when non-fatal exception occurs.

Possible solutions:

1. As @swankjesse said set your own uncaught exception handler which does not iniitate shutdown before initializing ACRA. This is probably not enough because app may enter into illegal state if fatal error occurs (eg. out of memory) but app won't crash and may behave unexpectedly. You can initiate shutdown after report is sent but in such case if non-fatal exceptions are reported you must distinguish them from fatal ones. So finally it is much tricky.
2. Try to modify OkHttp that it does not create new threads during shutdown. It should be possible to create custom thread pool executor (to be used in ConnectionPool and probably other places in OkHttp) which creates and starts thread instances at the beginning but later it reuses that instances instead of creating new ones on demand.
3. Use different http client.

[kaosko]

Thanks for digging up the relevant acra issue @koral--, missed that. I'll just use a snapshot.