The handling of InvocationContext will leak JBC's classloader. An InvocationContext gets stored in a ThreadLocal and doesn't get cleared. This is by design to avoid creating a new InvocationContext for each request, but it will have the effect of leaking the JBC classloader to whatever threads call into JBC.

Note that the fact that CacheImpl.invocationContextContainer is an instance field will not prevent the leak. When the CacheImpl is gc'd, the invocationContextContainer ThreadLocal will be gc'd as well. However, for each thread that invoked on the cache there will still be a ThreadLocal.ThreadLocalMap.Entry whose value field is a strong ref to the InvocationContext. If a particular thread's ThreadLocalMap happens to do some cleanup work (i.e. a rehash) it will detect that that ThreadLocal has been gc'd and expunge the Entry, but to allow the classloader to be gc'ed all such threads would have to do that, which is unlikely.

Solving this will be a pain, since clearing the ThreadLocal after each request will likely break a number of subtle things where code calls back into the cache. For example, Notifier would probably have to be changed.

Another option would be to have CacheImpl store a strong reference to every known context, which would go away on cleanup, although this would probably be expensive (at least for creation of the context)

Last, we could require that calling code hold the reference (somewhat cryptic, but this is an advanced feature).

I think the InvocationContextInterceptor should clean up the thread local entry as the invocation exits.

The only place this causes a problem is if there is a callback (like you said, with the notifier) and this triggers another cache operation which ends up resetting the thread local entry. This is less of a problem now since interceptors don't rely on the thread local once the call goes past the ICI (the IC is passed as a param). It is also less of a problem since (JBCACHE-1022) all notifications will now be emitted from the NotificationInterceptor, after which there is not much more than cleanup work remaining.

I think having the ICI clean up thread locals explicitly is more robust and maintainable than weak refs or calling code holding on to refs.

All the stuff in Option is appropriate for a client API, except I guess failSilently which AIUI is an implementation detail of putForExternalRead.

How about using interfaces for this -- InvocationContext and InvocationContextSPI, plus InvocationContextImpl? The InvocationContext interface just exposes the getters that make sense for a client, and via getOptionOverrides() makes the Option available.

If getOptionOverrides() is the only thing a client InvocationContext interface should expose, then maybe it makes more sense to change Cache so it exposes Cache.getOptionOverride() rather than getInvocationContext(). CacheSPI can expose getInvocationContext().