Stacktrace Missing after adding Listener for a Cache instance

[dawnwords]

Version of Cache2k：2.0.0.Final

How to reproduce:

1. Run the following two unit test cases.
2. testNpeWithoutListener passes but testNpeWithListener fails

package org.cache2k.test;

import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

import java.util.concurrent.TimeUnit;
import org.cache2k.Cache;
import org.cache2k.Cache2kBuilder;
import org.cache2k.event.CacheEntryRemovedListener;
import org.hamcrest.Matchers;
import org.junit.Test;

public class TestCache2k {
  @Test
  public void testNpeWithoutListener() {
    testAndVerifyStacktrace(Cache2kBuilder.of(String.class, String.class)
        .expireAfterWrite(10, TimeUnit.SECONDS)
        .entryCapacity(10)
        // no listener added
        .build());
  }

  @Test
  public void testNpeWithListener() {
    testAndVerifyStacktrace(Cache2kBuilder.of(String.class, String.class)
        .expireAfterWrite(10, TimeUnit.SECONDS)
        .entryCapacity(10)
        // listener added
        .addListener((CacheEntryRemovedListener<String, String>) (cache1, cacheEntry) -> {
        })
        .build());
  }


  private static void testAndVerifyStacktrace(Cache<String, String> cache) {
    try {
      final String value = null;
      cache.computeIfAbsent("npeKey", s -> value.substring(10));
      fail("this shall not pass");
    } catch (NullPointerException e) {
      assertThat(e.getStackTrace()[0].getClassName(), Matchers.is(TestCache2k.class.getName()));
    }
  }
}

Expectations: Both two cases pass, i.e. stacktrace within the callback function of computeIfAbsent should not be replaced by cache

Possible Reason: Stacktrace of the Exception thrown by the client callback function is replaced here

cache2k/cache2k-core/src/main/java/org/cache2k/core/BaseCache.java

Line 225 in 8a0024c

t.fillInStackTrace();

[cruftex]

Sorry for the inconsistence and thanks for the report.

I can reproduce the problem and just added your test case to the code base for future regression testing.

Cause:
When adding the listener, the processing switches to a more enhanced processing scheme. By design it is already prepared to do async processing. For example, async loading is supported already. In the future the entry processor might run in a separate thread as well. In this case we cannot throw the original exception directly but must wrap. The code which is causing the problem is generic and assumes that there is a wrapped exception, which is not the case for computeIfAbsent.

Basically the API semantic needs to be consistent.

Solution 1: We could consequently wrap into a EntryProcessingException identical to Cache.invoke. This would be generic and works in all scenarios. Its more consistent with the other cache methods, so replacing the computeIfAbsent with invoke does not lead to a change in the exception handling. Downdside: Exception wrapping is more overhead and exception semantics would be inconsistent with Map.computeIfAbsent then. There is an alternative ConcurrentMap interface that can be obtained via Cache.asMap(). All the compute methods here need to be fixed and don't wrap exceptions to comply with the ConcurrentMap interface.

Solution 2: Remove Cache.computeIfAbsent because its available in the ConcurrentMap interface. That only makes sense for version 3, since a major API change.

Solution 3: Don't wrap and make Cache.computeIfAbsent and ConcurrentMap.computeIfAbsent behave identically. That would be in line with the actual documentation.

I tend to do solution 3, because Cache.computeIfAbsent is there because its a very common use case and that should be supported directly and be most performant. Its just not aligning well with the rest of the advanced processing which assumes that exceptions are always wrapped.

I let that sit for a day or so, before I decide. Other ideas? Do you have any requirement that I may have overlooked?

[cruftex]

I corrected the problem and released an alpha update for verification. Thanks for reporting!
https://github.com/cache2k/cache2k/releases/tag/v2.1.2.Alpha