HighLevelRestClient (Java) - Client does not shut down after async requests

[codingchili (Robin Duda)]

Hello,

I'm unable to close the threadpool that is backing the HighLevelRestClient. Whenever my framework fails to deploy a part of the application or just for a graceful shutdown, I need the threadpool to shutdown to allow the JVM to terminate. This works as expected when I don't use the Elastic client libraries, or only use the synchronous requests.

Here is a reproducer, when invoking syncThenClose from the main method the application will terminate successfully. When invoking asyncThenClose the application won't terminate. 999 is a bogus port, elastic should not be running to generate the connection timeout error.

import org.apache.http.HttpHost;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.client.*;
import org.elasticsearch.client.indices.GetIndexRequest;

import java.io.IOException;

public class ElasticConnectionPoolTest {

    public static void main(String[] args) {
        // call one or the other and see if the jvm closes or not.
        //syncThenClose(); // scenario #1 process exits
        asyncThenClose();  // scenario #2 process doesn't exit.
    }

    private static void syncThenClose() {
        var client = new RestHighLevelClient(
                RestClient.builder(
                        new HttpHost("localhost", 999, "http")));

        var indices = client.indices();
        try {
            indices.exists(new GetIndexRequest("xm"), RequestOptions.DEFAULT);
        } catch (IOException e) {
            e.printStackTrace();
            try {
                client.close();
            } catch (IOException ex) {
                ex.printStackTrace();
            }
        }
    }

    private static void asyncThenClose() {
        var client = new RestHighLevelClient(
                RestClient.builder(
                        new HttpHost("localhost", 999, "http")));


        var indices = client.indices();
        indices.existsAsync(new GetIndexRequest("xm"), RequestOptions.DEFAULT, new ActionListener<>() {
            @Override
            public void onResponse(Boolean aBoolean) {
                throw new RuntimeException("use a different port");
            }

            @Override
            public void onFailure(Exception e) {
                e.printStackTrace();
                try {
                    client.close();
                } catch (Throwable ex) {
                    ex.printStackTrace();
                }
            }
        });
    }
}

Windows 10, Java 11.0.1,
org.Elasticsearch.client:Elasticsearch-rest-high-level-client:7.15.0 (and 7.13.2)

Note: I wasn't able to reproduce this as a junit4 test case due to how the test runner cleans up.

[codingchili (Robin Duda)]

The close method in CloseableHttpAsyncClientBase hangs forever.

public void close() {
        if (this.status.compareAndSet(CloseableHttpAsyncClientBase.Status.ACTIVE, CloseableHttpAsyncClientBase.Status.STOPPED) && this.reactorThread != null) {
            try {
                this.connmgr.shutdown();
            } catch (IOException var3) {
                this.log.error("I/O error shutting down connection manager", var3);
            }

            try {
                this.reactorThread.join();
            } catch (InterruptedException var2) {
                Thread.currentThread().interrupt();
            }
        }

    }

this.reactorThread.join() blocks indefinitely.

try {
    var close = new Thread(() -> {
        try {
            client.close();
        } catch (IOException ex) {
            ex.printStackTrace();
        }
    });
    close.start();
    Thread.sleep(2000);
    close.interrupt();
} catch (Throwable ex) {
    ex.printStackTrace();
}

If a new thread is created just to call close, I can then interrupt that thread and the application will exit - since the connectionmanager is at least closed before it calls reactorThread.join. No idea why it does this, I never used the apache http client before. reactorThread doesn't seem to be a daemon thread so the application can close if it is still running.

Since I already have an event loop (using Vert.x/Netty) I might as well run all requests using the blocking API's on those pools instead, although I did prefer the async methods for exception handling rather than try..catch.

[DavidTurner]

You're closing the client from the response-handling thread which sounds like a recipe for deadlock. Does it work to close the client from the main thread instead? For instance, use a future to block the main thread until the response is received, or dispatch the close call back onto your other event loop.

Would you share a complete thread dump of the process while it's stuck in the close() call?

[codingchili (Robin Duda)]

Ah yes, you're right. Dispatching the close onto my other event-loop works fine, this is what I should be doing anyways since I want to handle all responses on the same event loop.

Here is the thread dump,
https://gist.github.com/codingchili/aeb59b58bd43fabfd24c5a47f64be474

Thanks for your time

[DavidTurner]

@codingchili

Here is the thread dump,

Thanks, yes, that confirms it, we're effectively calling Thread.currentThread().join() which is a certain deadlock. Looks like newer versions of the Apache HTTP client libs use join(millis) instead and will interrupt any threads that remain.