fix for issue2742 null pointer exception

[ajaymaddi31]

fix for issue2742 null pointer exception

[AbfrmBlr]

IMHO the way to look into the problem is to understand the tiering model, and contract of flush method in AuthoritativeTier. In addition to the java docs on interfaces you can read about the whole tiering design here https://github.com/ehcache/ehcache3/blob/master/docs/src/docs/asciidoc/developer/design.tiering.adoc.

[ajaymaddi31]

we are getting valueHolder as null in getOrComputeIfAbsent method because the value for the key from cache is null and in the subsequent call to flush method throwing null pointer exception because of this issue.Now we are handling with this null check.After handling the null check we are not getting null pointer issue even after running the test case for 2 hours.

[ajaymaddi31]

Unit test case for NoopCachingTier(private static class in TieredStore.java) the method(getOrComputeIfAbsent) in NoopCachingTier is covered in TieredStoreTest.java, CachingTierDoesNotSeeAnyOperationDuringClear covers this method in NoopCachingTier.

Unit test case to Reproduce the NullPointerexception:- we are getting the null pointer exception in flush method in AbstractOffHeapStore.java class. In AbstractOffHeapStoreTest.java class has one test case named testFlushUpdateAccessStats covers flush method, by passing firstValuHolder and secondValueHolder. I wrote a similar test case(testFlushUpdateAccessStatsForNullPointerException) by passing the parameters(firstValueHolder,secondValueHolder) as null to reproduce the NullPointerException.

[chrisdennis]

Yeah, the test here doesn't seem to make sense given the context of the bug and the way the tiered store works. Can you schedule a 30 minute slot some time next week so we can discuss this, and I can explain what's going on with this code, why the null-check is really necessary, and what test needs to be written to reproduce the original NPE, and therefore prove the fix is correct.

[ajaymaddi31]

As per the yesterday's discussion added test case to cover the scenario for not calling flush method if the value is null.

[ajaymaddi31]

A gentle reminder, Please let us know if there are any review comments?

[chrisdennis]

A gentle reminder, Please let us know if there are any review comments?

https://user-images.githubusercontent.com/954331/212095088-555f7832-e160-4e4f-ba97-15579ce5ab7e.mov

[chrisdennis]

Before this can be merged the fix will need to be rebased to be a daggy fix against the commit that introduced the bug. I believe that's going to be the commit that introduces the concept of the NoopCachingTier. It should probably also be squashed to be a more logical commit stream.

[ajaymaddi31]

Opening new PR.

3114 is the replacement for this PR