• Bug
  • Status: Closed
  • 2 Major
  • Resolution: Fixed
  • ehcache-core
  • alexsnaps
  • Reporter: che
  • September 01, 2010
  • 0
  • Watchers: 0
  • July 27, 2012
  • December 13, 2010

Attachments

Description

Line 66-91: SelfPopulatingCache.java

This code assumed implicitly that LockTimeoutException would only be throw from: Element element = super.get(key);

and LockTimeoutException would never be throw from: Object value = factory.createEntry(key);

This is not a safe assumption due to custom implementations of factory.createEntry(key). We implemented this factory method in a nested fashion, i.e. category has sub-category inside, sub-category has product, and product has SKU etc. This createEntry(key) actually access its children which use ehcache. When the any child failed and throw the LockTimeoutException (such as due timeout limit), this LockTimeoutException is re-throw to its parent where factory.createEntry(key) is called. Due to the above unsafe assumption, the lock of key (for the parent) is never release, The parent only re-throw the LockTimeoutException due to code line 78-82.

This issue caused accumulated stuck thread (hence jvm failure) before we set a timeout. After we set a timeout, we saw massive LockTimeoutException leading to one section of the site frozen where ehcache is used.

Here is the code I am referring too, also attached the full java class source code: Line 66-91: SelfPopulatingCache.java public Element get(final Object key) throws LockTimeoutException {

    try {
        //if null will lock here
        Element element = super.get(key);
        if (element == null) {
            // Value not cached - fetch it
            Object value = factory.createEntry(key);
            element = makeAndCheckElement(key, value);
            put(element);
        }
        return element;
    } catch (LockTimeoutException e) {
        //do not release the lock, because you never acquired it
        String message = "Timeout after " + timeoutMillis + " waiting on another thread " +
                "to fetch object for cache entry \"" + key + "\".";
        throw new LockTimeoutException(message, e);

    } catch (final Throwable throwable) {
        // Could not fetch - Ditch the entry from the cache and rethrow

        //release the lock you acquired
        put(new Element(key, null));
        throw new CacheException("Could not fetch object for cache entry with key \"" + key + "\".", throwable);
    }
}

Comments

Steve Harris 2010-09-21

Is he right?

Fiona OShea 2010-10-28

Any updates?

Alexander Snaps 2010-11-03

We had this chat on the transparency mailing list, and I don’t agree with the issue. I think it is the role of the user’s net.sf.ehcache.constructs.blocking.CacheEntryFactory.createEntry implementation to handle the issue, not the SelfPopulatingCache. Now I can be wrong, or simply in disagreement with everybody else. Now, should people agree with me, then we might want to document that in the net.sf.ehcache.constructs.blocking.CacheEntryFactory.createEntry JavaDoc. wdyt ?

Alexander Snaps 2010-12-13

Should I merge this on 2.3.1 ?