I think it would be easier to read if the count were the first parameter.

Maybe rename to compareAndRun? I think this name helps with the intuition from "compare and swap".
"counter" is used too much so maybe drop it if there's a parameter with the name counter already.

Runnable -> CheckedRunnable, to avoid worrying about the checked/unchecked exceptions at this low level.

Your analogy of AtomicInteger makes sense. The name compareAndRun also sounds good and to the point. The count parameter naturally should become the first parameter after the method name change to follow the convention of compareAndSet. Thanks!

I decided to keep Runnable instead of changing it to CheckedRunnable. In fact, I started with CheckedRunnable, but I think it has a few disadvantages:

1. If we don't handle the exception of CheckedRunnable, the method would have a signature like:
   public <E extends Exception> boolean compareAndRun(long count, CheckedRunnable<E> runnable) throws E
   It forces its caller to handle exceptions even when the Runnable may not throw any exception (as in the NativePrivilegesStore case)
2. If we handle the exception inside the method and only bubble up RuntimeException, it will make the method easier to call because no exception handling is forced onto the caller. However, the method itself has no knowledge about he Runnable so it cannot perform meaningful handling other than maybe wrap it with a RuntimeException. It works, but makes it hard if the caller cares about the real exception and wants to handle it.

My understanding is that CheckedRunnable is better when possible exception types are limited and the method knows well how to handle (or ignore) them. Therefore, I prefer to just using Runnable. It works similar to above item 2, but giving caller full control of how exception should or should not be handled.

Okay, sounds good to me. It's not going to be a big difference in practice anyway.

As discussed, we both agree that encapsulating a cache inside this class is the wrong abstraction (which is InvalidationCountingCacheWrapper), although I struggle to put it in words in a comment.

I still tend to have more ambitious hopes for this cache invalidation pattern, but I very much fear we can sunk in it and not achieve much.

Overall, I think abstraction here is good enough!
But its rich history is showing in the description and in the names. We should polish these.

I would think about this class as a special type of atomic integer, which has a fancy method "run if integer constant", such that the run block of code cannot overlap with changes (increments) to the integer.
It's hard to explain it's usage in the caching context, so I would steer away from it completely.
For example, we rely that changes to the underlying data (eg api key docs) are performed before calling increment and that we call getCount before we retrieve the underlying data, so that we ensure the sequence modify data -> increment -> getCount -> read data -> cache data .

My explicit suggestion is to rename to CounterCompare, or anythign else that inspires that the runnable and the counter cannot overlap.

Another suggestion, if you think this is unlikely to be used outside of caching contexts, it might be useful to make it a static inner class of Cache.

After some thoughts and taking your advice onboard, I came up with the name LockingAtomicCounter. It is an unexciting name, but I think it conveys the major idea here: a counter with RWLock to ensure its value stays the same until the reading side finishes its task.

I think your analogy of AtomicInteger is a great one. In concept, compareAndSet and compareAndRun are very similiar. They are both executing some operation if the counter value matches the expectation. The only difference is that the operation of the former is a single setValue, but the later can have an arbitrary Runnable. This is also the reason that I went with the name of LockingAtomicCounter, which I hope makes its behaviour easier to understand.

I updated the comments accordingly.

As for the location of this class, I'd prefer to have it stay inside xpack security for the time being because:

1. The usages that I can think of for now are security caches
2. It in theory operates at a higher level than a single cache, e.g. ApiKeyDocCache is a wrapper for two vanila caches.
3. The way it ends right now really does not say anything about caching anymore. So I have hope that it could still be useful elsewhere.

I came up with the name LockingAtomicCounter

SGTM