KVCache dose not closes properly

[DimaGolomozy]

Hi,
Running small code that starts KVCache then stops it and then exits that main. The JVM is still running
and not closes properly.
Only if I put System.exit in the end the JVM shuts down.

public static void main(String[] args) throws NamingException
    {
        KVCache kvCache;
        kvCache = KVCache.newCache(Consul.builder().build().keyValueClient(), "dataplatform/applicationConfigTest", 10);
        try {
            kvCache.start();
        } catch (Exception ex) {
            throw new RuntimeException();
        }

        try {
            if (!kvCache.awaitInitialized(10, TimeUnit.SECONDS))
                throw new RuntimeException();
        } catch (InterruptedException ie) {
            throw new RuntimeException();
        }

        try {
            kvCache.stop();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

[KekSfabrik]

I can confirm this (on the latest, 0.12.4, from mvnrepository.com):
OkHttp Dispatcher, OkHttp ConnectionPool and Okio Watchdog stay alive and never seem to be shut down - my ServiceHealthCache Instances take about 60s to shut down after receiving stop() (after i removed all Listeners) - that's bad already (realistically the App will be killed off hard (kill -9) after ~15s or so of receiving a shutdown signal from the outside) and the KVCache Instance never shuts down its threads.
If you need anything to get this fixed or can't reproduce it let me know

[Hronom]

+1 confirm it on versions 0.12.3 and 0.12.4, same behaviour.

[gjesse]

been looking into this some - it's not actually a problem with KVCache, but with the okHttp services. For example, the following code triggers the same behavior, no kvCache needed.

 public static void main(String[] args) throws NamingException, InterruptedException {

        final AtomicReference<BigInteger> latestIndex = new AtomicReference<BigInteger>(null);

        final CountDownLatch latch = new CountDownLatch(1);
        final Consul consul = Consul.builder().build();
        KeyValueClient kvClient = consul.keyValueClient();

        kvClient.putValue("/key", "value");

        kvClient.getValues("/key", QueryOptions.BLANK, new ConsulResponseCallback<List<Value>>() {
            @Override
            public void onComplete(ConsulResponse<List<Value>> consulResponse) {
                latestIndex.set(consulResponse.getIndex());
                latch.countDown();
            }

            @Override
            public void onFailure(Throwable throwable) {
                latch.countDown();
            }
        });

        System.out.println("sent request");

        latch.await();

        System.out.println("request completed");
    }

There is an executorService created by the dispatcher library here: https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/Dispatcher.java#L64-L65 that is not a daemon, and as such block the jvm from shutting down.

There's a few options

1. create a dispatcher executor that uses a daemon thread. This is pretty straightforward and seems harmless enough. I've tested this locally and it looks fine.
2. create our own dispatcher and add lifecycle methods into Consul.java to shut it down. This is a bit annoying for any existing users to implement
3. submit a PR to okhttp to default to a daemon thread, which might take a while, if they even accept it. they might have good reasons to make this a non-daemon executorService.

I'll go ahead and push up a PR for 1 above for immediate relief.

[rickfast]

Fixed via 3f6cacd. Thanks @gjesse