adding async expiration listener outside of builder causes problems

[massfords]

The behavior I'm seeing in my app is that the async expiration listener that I add after building the ExpiringMap doesn't receive its callbacks and some of the items in the map don't expire.

You can see a unit test for this on my fork that recreates the problem.

Adding in Builder and After works

ExpiringMap<String, String> map = ExpiringMap.builder()
                .expiration(100, TimeUnit.MILLISECONDS)
                .asyncExpirationListener((thekey, thevalue) -> {
                    System.out.println("builder one called");
                 })
                .build();
map.addAsyncExpirationListener((thekey, thevalue) -> {
     System.out.println("after builder called");
});

Adding only after the builder doesn't work

ExpiringMap<String, String> map = ExpiringMap.builder()
                .expiration(100, TimeUnit.MILLISECONDS)
                .build();
map.addAsyncExpirationListener((thekey, thevalue) -> {
     System.out.println("after builder called");
});

In this case, the expiration listener isn't called and I'm also seeing that some items don't expire from the map. In all cases I'm adding the listener immediately after building the map and before putting any items in it.

Note that in my branch I added priority to the TestNG methods. If I run the whole suite and your existing tests run first then everything works. If I just run my test then it fails. If I sequence the tests such that mine run first then the failure is present. I haven't dug into the ExpiringMap or its builder to see what might be sensitive to the ordering of the tests but I don't see anything shared between them the way the existing tests were written.

[massfords]

The source of the problem is in ExpiringMap's private constructor where it only initializes the static LISTENER_SERVICE and THREAD_FACTORY if the builder has async listeners.

    if (LISTENER_SERVICE == null && builder.asyncExpirationListeners != null) {
      synchronized (ExpiringMap.class) {
        if (LISTENER_SERVICE == null) {
          LISTENER_SERVICE = (ThreadPoolExecutor) Executors.newCachedThreadPool(
              THREAD_FACTORY == null ? new NamedThreadFactory("ExpiringMap-Listener-%s") : THREAD_FACTORY);
        }
      }
    }

This explains why the tests need to be ordered they way they are.

[jhalterman]

Would be happy to take a PR for this.

[r0b0ji]

Would be happy to take a PR for this.

Created a PR https://github.com/jhalterman/expiringmap/pull/72

[simenflatby]

For other hitting this bug before the PR #72 is merged and a new version is released, a workarround is to call the asyncExpirationListeners of the builder with an empty list before calling build(). This will initialize the LISTENER_SERVICE. You can then use ExpiringMap#addAsyncExpirationListener as expected.

Example of an extension function in Kotlin that does this:

private fun <K, V> ExpiringMap.Builder<K, V>.initializeAsyncExpirationListeners(): ExpiringMap.Builder<K, V> {
        asyncExpirationListeners(emptyList<ExpirationListener<K, V>>())
        return this
}

and usage:

val map = ExpiringMap.builder()
    .expiration(100, TimeUnit.MILLISECONDS)
    .initializeAsyncExpirationListeners()
    .build<String, String>()

map.addAsyncExpirationListener { key: String, value: String -> TODO() }

This works on version 0.5.9.

[jhalterman]

Closed by #72

[jhalterman]

This was released in 0.5.10.