You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by de...@apache.org on 2017/09/17 12:29:38 UTC

[myfaces-trinidad] branch andy-trinidad-2468 created (now d19f383)

This is an automated email from the ASF dual-hosted git repository.

deki pushed a change to branch andy-trinidad-2468
in repository https://gitbox.apache.org/repos/asf/myfaces-trinidad.git.


      at d19f383  Tweaking empty style nodes handling, at Blake's suggestion.  Instead of failing silently (or NPE'ing), let's fail in a more obvious way.

This branch includes the following new commits:

     new 8bda907  Branching from Trinidad trunk for work on issue 2468
     new bbb99f0  Checkpoint: temporarily parking fix for TRINIDAD-2468.
     new 7077816  Tweaking InterruptedException handling.
     new d19f383  Tweaking empty style nodes handling, at Blake's suggestion.  Instead of failing silently (or NPE'ing), let's fail in a more obvious way.

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


-- 
To stop receiving notification emails like this one, please contact
['"commits@myfaces.apache.org" <co...@myfaces.apache.org>'].

[myfaces-trinidad] 03/04: Tweaking InterruptedException handling.

Posted by de...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

deki pushed a commit to branch andy-trinidad-2468
in repository https://gitbox.apache.org/repos/asf/myfaces-trinidad.git

commit 70778168ed203b4933f5e031062b11d45ab7e28f
Author: Andy Schwartz <an...@apache.org>
AuthorDate: Wed Apr 16 15:11:30 2014 +0000

    Tweaking InterruptedException handling.
---
 .../style/cache/FileSystemStyleCache.java          | 27 +++++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java b/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
index 02734ff..29ffcb3 100644
--- a/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
+++ b/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
@@ -530,16 +530,19 @@ public class FileSystemStyleCache implements StyleProvider
     }
     catch (InterruptedException ie)
     {
-      // Important to restore interrupted state, even if we are going to
-      // throw a runtime exception.
-      interrupted = true;
-      
       // Our thread was either interrupted just before the get() call, or
       // while waiting inside of the get().  Our retry attempt failed.  We
       // could carry on without a style sheet, but this will only lead to
       // confusion, so we choose to fail visibly instead and hope that things
       // go better on the next request.
       _logAndRethrowEntryGetFailure(context, document, ie, "STYLE_ENTRY_RETRIEVAL_INTERRUPTED");
+      
+      // Note that we could call Thread.currentThread().interrupt() here, but choose
+      // not to do so because a) we are effectively ending the request by throwing
+      // an exception and b) marking the thread as interrupted seems to interfere
+      // with MyFaces error handling.  If thread is marked as interrupted, I am
+      // not seeing the MyFaces-generated error page - just an empty status 200
+      // response.
     }
     catch (ExecutionException ee)
     {
@@ -556,7 +559,7 @@ public class FileSystemStyleCache implements StyleProvider
       if (interrupted)
       {
         Thread.currentThread().interrupt();
-      }
+      }      
     }
 
     return null;    
@@ -601,6 +604,11 @@ public class FileSystemStyleCache implements StyleProvider
    * 
    * The message is formatted with a single parameter: the name of the target
    * style sheet that we were attempting to retrieve.
+   * 
+   * @param contex the current style context
+   * @param document the style sheet document
+   * @param e an exception thrown by Future.get().  This is typically
+   *   either ExecutionException or InterruptedException
    */
   private void _logAndRethrowEntryGetFailure(
     StyleContext context,
@@ -615,6 +623,7 @@ public class FileSystemStyleCache implements StyleProvider
 
     String targetName = getTargetStyleSheetName(context, document);
     _LOG.severe(message, targetName);
+    _LOG.fine(e);
 
     Throwable cause = e.getCause();
     if (cause instanceof RuntimeException)
@@ -625,10 +634,16 @@ public class FileSystemStyleCache implements StyleProvider
     {
       throw (Error)cause;
     }
-    else
+    else if (cause instanceof Exception)
     {
       throw new IllegalStateException(cause);
     }
+    else
+    {
+      // This is the InterruptedException case, since InterruptedExceptions
+      // don't have a cause.
+      throw new IllegalStateException(message);
+    }
   }
 
   /**

-- 
To stop receiving notification emails like this one, please contact
"commits@myfaces.apache.org" <co...@myfaces.apache.org>.

[myfaces-trinidad] 01/04: Branching from Trinidad trunk for work on issue 2468

Posted by de...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

deki pushed a commit to branch andy-trinidad-2468
in repository https://gitbox.apache.org/repos/asf/myfaces-trinidad.git

commit 8bda907487df2c02cccc2e68ee90472fb8d0f600
Author: Andy Schwartz <an...@apache.org>
AuthorDate: Wed Apr 16 09:27:34 2014 +0000

    Branching from Trinidad trunk for work on issue 2468

-- 
To stop receiving notification emails like this one, please contact
"commits@myfaces.apache.org" <co...@myfaces.apache.org>.

[myfaces-trinidad] 04/04: Tweaking empty style nodes handling, at Blake's suggestion. Instead of failing silently (or NPE'ing), let's fail in a more obvious way.

Posted by de...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

deki pushed a commit to branch andy-trinidad-2468
in repository https://gitbox.apache.org/repos/asf/myfaces-trinidad.git

commit d19f383c44cddb585a69aed962ac021a667b2faa
Author: Andy Schwartz <an...@apache.org>
AuthorDate: Thu May 15 14:44:42 2014 +0000

    Tweaking empty style nodes handling, at Blake's suggestion.  Instead of failing silently (or NPE'ing), let's fail in a more obvious way.
---
 .../style/cache/FileSystemStyleCache.java              | 18 +++++++++++++++++-
 .../trinidadinternal/resource/LoggerBundle.xrts        |  3 +++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java b/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
index 29ffcb3..e5e66f8 100644
--- a/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
+++ b/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
@@ -785,7 +785,12 @@ public class FileSystemStyleCache implements StyleProvider
     List<StyleNode> styleNodes = _getStyleContextResolvedStyles(context, document);
     
     if (styleNodes.isEmpty())
-      return null;
+    {
+      // Historically we have failed silently here.  After further thought, we
+      // decided it would be better to make some noise so that folks know
+      // that something has gone wrong.
+      _throwEmptyStyleNodes(context, document);
+    }
 
     // This code fills in the <Selector, Style> resolvedSelectorStyleMap map. 
     // We use _reusableStyleMap to reuse the Style objects when possible
@@ -843,6 +848,17 @@ public class FileSystemStyleCache implements StyleProvider
                                    shortStyleClassMap,  _isCompressStyles(context), resolvedSelectorStyleMap);
     return new Entry(uris, styles, icons, skinProperties);
   }
+  
+  private void _throwEmptyStyleNodes(
+    StyleContext context,
+    StyleSheetDocument document
+    )
+  {
+    String targetName = getTargetStyleSheetName(context, document);
+    String message = _LOG.getMessage("STYLE_ENTRY_CREATION_FAILED_NO_STYLES",
+                                     new Object[] {targetName});
+    throw new IllegalStateException(message);
+  }
 
   /**
    * Look in the entry cache for a compatible entry.
diff --git a/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts b/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
index 939522d..f47ea1b 100644
--- a/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
+++ b/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
@@ -1242,4 +1242,7 @@ The skin {0} specified on the requestMap will be used even though the consumer''
 
 <resource key="STYLE_ENTRY_CREATION_FAILED">Creation of style sheet {0} has failed.  Will retry on next request.</resource>
 
+<resource key="STYLE_ENTRY_CREATION_FAILED_NO_STYLES">Creation of style sheet {0} has failed.  No styles found.</resource>
+
+
 </resources>

-- 
To stop receiving notification emails like this one, please contact
"commits@myfaces.apache.org" <co...@myfaces.apache.org>.

[myfaces-trinidad] 02/04: Checkpoint: temporarily parking fix for TRINIDAD-2468.

Posted by de...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

deki pushed a commit to branch andy-trinidad-2468
in repository https://gitbox.apache.org/repos/asf/myfaces-trinidad.git

commit bbb99f0ef90bf414c6922acd9946138125ab293a
Author: Andy Schwartz <an...@apache.org>
AuthorDate: Wed Apr 16 09:36:47 2014 +0000

    Checkpoint: temporarily parking fix for TRINIDAD-2468.
---
 .../style/cache/FileSystemStyleCache.java          | 355 ++++++++++++++++++---
 .../trinidadinternal/resource/LoggerBundle.xrts    |   5 +
 2 files changed, 317 insertions(+), 43 deletions(-)

diff --git a/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java b/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
index 4b3ffbf..02734ff 100644
--- a/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
+++ b/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
@@ -38,9 +38,18 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+
+import java.util.concurrent.FutureTask;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
 import org.apache.myfaces.trinidad.context.AccessibilityProfile;
 import org.apache.myfaces.trinidad.context.LocaleContext;
 import org.apache.myfaces.trinidad.context.RenderingContext;
@@ -50,6 +59,7 @@ import org.apache.myfaces.trinidad.skin.Skin;
 import org.apache.myfaces.trinidad.style.Selector;
 import org.apache.myfaces.trinidad.style.Style;
 import org.apache.myfaces.trinidad.style.Styles;
+import org.apache.myfaces.trinidad.util.Args;
 import org.apache.myfaces.trinidad.util.ArrayMap;
 import org.apache.myfaces.trinidad.util.CollectionUtils;
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent;
@@ -348,7 +358,14 @@ public class FileSystemStyleCache implements StyleProvider
   // And Entry contains the style sheet URI.
   private Entry _getEntry(StyleContext context)
   {
-    ConcurrentMap<Key, Entry> cache = null;
+    // Our main entry map is Future-based to ensure that we will not have multiple
+    // threads attempting to create the same Entry (and, more importantly, writing
+    // the Entry's style sheets) in parallel.  This replaces the previous implementation,
+    // which was not thread safe and ran the risk of populating our entry cache with
+    // corrupt instances.  See JCIP section 5.6, "Builing an efficient, scalable result
+    // cache" for more details on this approach.
+    ConcurrentMap<Key, Future<Entry>> cache = null;
+
     ConcurrentMap<Object, Entry> entryCache = null;
     StyleSheetDocument document = null;
     Map<String, String> shortStyleClassMap = null;
@@ -388,7 +405,7 @@ public class FileSystemStyleCache implements StyleProvider
       // throughout the entire request, to avoid adding bogus entries
       // to a new re-allocated cache.
       if (_cache == null)
-        _cache = new ConcurrentHashMap<Key, Entry>();
+        _cache = new ConcurrentHashMap<Key, Future<Entry>>();
       
       if (_entryCache == null)
         _entryCache = new ConcurrentHashMap<Object, Entry>(19);
@@ -421,47 +438,219 @@ public class FileSystemStyleCache implements StyleProvider
     {
       _LOG.finest("FileSystemStyleCache's Key's hashCode is ", key.hashCode());
     }
-    Entry entry = _getEntry(cache, key, checkModified);
+
+    Entry entry = _getEntry(context, document, cache, key, checkModified);
     if (entry != null)
       return entry;
 
     // Next see if this is an entry which is compatible with this request
-    entry = _getCompatibleEntry(context, document, cache, key, entryCache, checkModified);
+    DerivationKey derivationKey = _getDerivationKey(context, document);
+    entry = _getCompatibleEntry(cache, key, derivationKey, entryCache, checkModified);
 
     if (entry != null)
       return entry;
 
     // If we didn't find an entry in the cache, create a new entry
     // This generates the CSS file.
-    return _createEntry(context,
+    entry = _createEntrySafely(context,
                         document,
                         cache,
                         key,
-                        entryCache,
                         shortStyleClassMap,
                         namespacePrefixes,
-                        checkModified, 
+                        checkModified,
                         isDirty);
+
+    // Also, cache the new entry in the DerivationKey-based entry cache
+    entryCache.put(derivationKey, entry);
+    
+    // just in case, clear the dirty flag.
+    RenderingContext arc = RenderingContext.getCurrentInstance();
+    Skin skin = arc.getSkin();
+    skin.setDirty(false);
+    
+    return entry;
   }
 
   private Entry _getEntry(
-    ConcurrentMap<?, Entry> cache,
-    Object                  key,
-    boolean                 checkModified
+    StyleContext context,
+    StyleSheetDocument document,
+    ConcurrentMap<Key, Future<Entry>> cache,
+    Key key,
+    boolean checkModified
     )
   {
-    Entry entry = cache.get(key);
-    if (entry == null)
+    Future<Entry> f = cache.get(key);
+    Entry entry = _getEntryFromFuture(context, document, cache, key, f);
+    if ((entry != null) && !_validateEntry(entry, checkModified))
+    {
+      // atomically remove the key from the cache if it currently points to the entry
+      cache.remove(key, entry);
+      entry = null;
+    }
+    
+    return entry;
+  }
+
+  /**
+   * Resolves the Future<Entry>, if non-null.
+   * 
+   * @return null if f is null, or the Future's resolved Entry value otherwise.
+   */
+  private Entry _getEntryFromFuture(
+    StyleContext context,
+    StyleSheetDocument document,
+    ConcurrentMap<Key, Future<Entry>> cache,
+    Key key,
+    Future<Entry> f)
+  {
+    if (f == null)
     {
+      // null is a valid argument for the Future, since the cache key may not
+      // have been put yet.
       return null;
     }
 
+    // If the current thread is already interrupted, our call to Future.get() will
+    // fail with an InterruptedException even if the Future result has already been
+    // computed.  We want to get the result even if the current thread has been
+    // interrupted, so clear out the interrupted state temporarily while we make
+    // the get() call.
+    boolean interrupted = Thread.interrupted();
+
+    try
+    {
+      // Even though we cleared out the interrupted state above, our thread might be
+      // re-interrupted while we are waiting for a second thread to finish writing
+      // the style sheet files to the file system (which can be slow).  We don't
+      // want to give up so easily though... in the event that we are interrupted
+      // while waiting on another thread, let's retry the Future.get() one time
+      // in the hopes that this will be sufficient to allow us to get our Entry.
+      return _getEntryFromFutureWithRetry(f, 1);
+    }
+    catch (InterruptedException ie)
+    {
+      // Important to restore interrupted state, even if we are going to
+      // throw a runtime exception.
+      interrupted = true;
+      
+      // Our thread was either interrupted just before the get() call, or
+      // while waiting inside of the get().  Our retry attempt failed.  We
+      // could carry on without a style sheet, but this will only lead to
+      // confusion, so we choose to fail visibly instead and hope that things
+      // go better on the next request.
+      _logAndRethrowEntryGetFailure(context, document, ie, "STYLE_ENTRY_RETRIEVAL_INTERRUPTED");
+    }
+    catch (ExecutionException ee)
+    {
+      // Remove the Future<Entry> from our cache on the hopes that this is
+      // a transient problem that will not show up again if we try with a 
+      // fresh Future on a subsequent request.
+      cache.remove(key, f);
+      
+      // And fail loudly, sincew e cannot carry on without a style sheet.
+      _logAndRethrowEntryGetFailure(context, document, ee, "STYLE_ENTRY_CREATION_FAILED");
+    }
+    finally
+    {
+      if (interrupted)
+      {
+        Thread.currentThread().interrupt();
+      }
+    }
+
+    return null;    
+  }
+
+  /**
+   * Attempts to resolve a future.  If retryCount is > 1, will re-attempt in the
+   * event that an InterruptedException is thrown.
+   * 
+   * @param f The Future to resolve
+   * @param retryCount The number of retry attempts if interrupted
+   * @return the Entry value of the Future<Entry>
+   * @throws InterruptedException if the Future.get() continues to throw
+   *   InterruptedException of retryCount retries, the InterruptedException
+   *   is rethrown out of this method.
+   * @throws ExecutionException if thrown by Future.get()
+   */
+  private Entry _getEntryFromFutureWithRetry(Future<Entry> f, int retryCount)
+    throws InterruptedException, ExecutionException
+  {
+    Args.notNull(f, "f");
+
+    try
+    {
+      return f.get();
+    }
+    catch (InterruptedException ie)
+    {
+      if (retryCount > 0)
+      {
+        return _getEntryFromFutureWithRetry(f, retryCount - 1);
+      }
+      
+      throw ie;
+    }
+  }
+
+  /**
+   * Logs a message in the event that we are not able to retrieve the
+   * Entry instance and then throws the exception (wrapped in
+   * an runtime exception) or its cause (similarly wrapped if necessary).
+   * 
+   * The message is formatted with a single parameter: the name of the target
+   * style sheet that we were attempting to retrieve.
+   */
+  private void _logAndRethrowEntryGetFailure(
+    StyleContext context,
+    StyleSheetDocument document,
+    Exception e,
+    String message)
+  {
+    Args.notNull(context, "context");
+    Args.notNull(document, "document");
+    Args.notNull(e, "e");
+    Args.notNull(message, "message");
+
+    String targetName = getTargetStyleSheetName(context, document);
+    _LOG.severe(message, targetName);
+
+    Throwable cause = e.getCause();
+    if (cause instanceof RuntimeException)
+    {
+      throw (RuntimeException)cause;
+    }
+    else if (cause instanceof Error)
+    {
+      throw (Error)cause;
+    }
+    else
+    {
+      throw new IllegalStateException(cause);
+    }
+  }
+
+  /**
+   * Validates that the specified entry is valid.
+   * 
+   * @param entry the Entry to validate
+   * @param checkModified whether we should check for file system modifications
+   * 
+   * @return true if the entry is valid, false a) checkModified is true and
+   *   b) the files corresponding to the entry have been removed
+   */
+  private boolean _validateEntry(Entry entry, boolean checkModified)
+  {
+    Args.notNull(entry, "entry");
+
+    boolean valid = true;
+
     if (checkModified)
     {
       List<String> uris = entry.uris;
       assert uris != null && !uris.isEmpty();
 
-      boolean valid = true;
       List<File> existing = new LinkedList<File>();
       // Make sure the entry's file exists.  If it no longer
       // exists, we remove the entry from the cache
@@ -474,6 +663,9 @@ public class FileSystemStyleCache implements StyleProvider
         }
         else
         {
+          // Even though we know that we are invalid at this point, keep
+          // on looping so that we can gather up all of the existing files,
+          // which we will delete below.
           valid = false;
         }
       }
@@ -481,15 +673,50 @@ public class FileSystemStyleCache implements StyleProvider
       if (!valid)
       {
         _deleteAll(existing);
-        
-        // atomically remove the key from the cache if it currently points to the entry
-        cache.remove(key, entry);
+      }
+    }
 
-        return null;
+    return valid;
+  }
+
+  /**
+   * Creates and returns an Entry instance in a thread-safe manner.
+   */
+  private Entry _createEntrySafely(
+    final StyleContext                 context,
+    final StyleSheetDocument           document,
+    final ConcurrentMap<Key, Future<Entry>> cache,
+    final Key                          key,
+    final Map<String, String>          shortStyleClassMap,
+    final String[]                     namespacePrefixes,
+    final boolean                      checkModified,
+    final boolean                      isDirty)
+  {
+    // See JCIP 5.6 if you are confused by what we are doing here.
+    Callable<Entry> entryCreator = new Callable<Entry>() {
+
+      @Override
+      public Entry call()
+      {
+        return _createEntry(context,
+                            document,
+                            shortStyleClassMap,
+                            namespacePrefixes,
+                            checkModified,
+                            isDirty);
       }
+    };
+
+    FutureTask<Entry> ft = new FutureTask<Entry>(entryCreator);
+    Future<Entry> f = cache.putIfAbsent(key, ft);
+    
+    if (f == null)
+    {
+      f = ft;
+      ft.run();
     }
 
-    return entry;
+    return _getEntryFromFuture(context, document, cache, key, f);
   }
 
   /**
@@ -527,9 +754,6 @@ public class FileSystemStyleCache implements StyleProvider
   private Entry _createEntry(
     StyleContext                 context,
     StyleSheetDocument           document,
-    ConcurrentMap<Key, Entry>    cache,
-    Key                          key,
-    ConcurrentMap<Object, Entry> entryCache,
     Map<String, String>          shortStyleClassMap,
     String[]                     namespacePrefixes,
     boolean                      checkModified,
@@ -602,19 +826,7 @@ public class FileSystemStyleCache implements StyleProvider
     // like browser, agent, locale, direction.
     Styles styles = new StylesImpl(namespacePrefixes, _STYLE_KEY_MAP,
                                    shortStyleClassMap,  _isCompressStyles(context), resolvedSelectorStyleMap);
-    Entry entry = new Entry(uris, styles, icons, skinProperties);
-    cache.put(key, entry);
-
-    // Also, cache the new entry in the entry cache
-    DerivationKey derivationKey = _getDerivationKey(context, document);
-    entryCache.put(derivationKey, entry);
-    
-    // just in case, clear the dirty flag.
-    RenderingContext arc = RenderingContext.getCurrentInstance();
-    Skin skin = arc.getSkin();
-    skin.setDirty(false);
-
-    return entry;
+    return new Entry(uris, styles, icons, skinProperties);
   }
 
   /**
@@ -623,21 +835,32 @@ public class FileSystemStyleCache implements StyleProvider
    * same StyleSheetNodes.
    */
   private Entry _getCompatibleEntry(
-    StyleContext                 context,
-    StyleSheetDocument           document,
-    ConcurrentMap<Key, Entry>    cache,
+    ConcurrentMap<Key, Future<Entry>> cache,
     Key                          key,
+    DerivationKey                derivationKey,
     ConcurrentMap<Object, Entry> entryCache,
     boolean                      checkModified
     )
   {
-    DerivationKey derivationKey = _getDerivationKey(context, document);
-    Entry entry = _getEntry(entryCache, derivationKey, checkModified);
+    Entry entry = entryCache.get(derivationKey);
+    if ((entry != null) && !_validateEntry(entry, checkModified))
+    {
+      entryCache.remove(derivationKey, entry);
+      entry = null;
+    }
+
     if (entry != null)
     {
-      // If we've got a compatible entry, cache it
-      cache.put(key, entry);
+      // If we've find a matching entry, store it in the main Key-based cache.
+      cache.putIfAbsent(key, new ResolvedFuture<Entry>(entry));
+      
+      // If the cache already contains an entry (ie. putIfAbsent returns a 
+      // non-null value), this means that the Key-based cached and the 
+      // DerivationKey-based cache will contain different Entry instances.
+      // This is somewhat unexpected but not necessarily fatal, so we don't 
+      // take any special action for this case.
     }
+
     return entry;
   }
 
@@ -1655,6 +1878,49 @@ public class FileSystemStyleCache implements StyleProvider
       return _getWriter(outputFile);
     }
   }
+
+  // A Future implementation that holds an already-resovled value.
+  private static final class ResolvedFuture<V> implements Future<V>
+  {
+    public ResolvedFuture(V value)
+    {
+      _value = value;
+    }
+
+    @Override
+    public boolean cancel(boolean mayInterruptIfRunning)
+    {
+      return false;
+    }
+
+    @Override
+    public boolean isCancelled()
+    {
+      return false;
+    }
+
+    @Override
+    public boolean isDone()
+    {
+      return true;
+    }
+
+    @Override
+    public V get()
+      throws InterruptedException, ExecutionException
+    {
+      return _value;
+    }
+
+    @Override
+    public V get(long timeout, TimeUnit unit)
+      throws InterruptedException, ExecutionException, TimeoutException
+    {
+      return get();
+    }
+    
+    private final V _value;
+  }
   
   private final String _targetPath; // The location of the cache
 
@@ -1672,8 +1938,11 @@ public class FileSystemStyleCache implements StyleProvider
    * be the same between FileSystemStyleCache$StylesImpl instances, so they should be shared. */
   private ConcurrentMap<String, Selector> _reusableSelectorMap;
   
-  /** The cache of style sheet URIs */
-  private ConcurrentMap<Key, Entry> _cache;
+  /**
+   * The main Entry cache.  Future-based to ensure that only a single thread
+   * attempts to create an Entry for a specific Key at a time.
+   */
+  private ConcurrentMap<Key, Future<Entry>> _cache;
 
   /**
    * We cache Entry objects, hashed by DerivationKey (ie.
diff --git a/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts b/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
index 0b33b07..939522d 100644
--- a/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
+++ b/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
@@ -1237,4 +1237,9 @@ The skin {0} specified on the requestMap will be used even though the consumer''
 <resource key="EXC_PLEASE_SEE_ERROR_LOG">An error occurred while processing the request. For more information, please see the server's error log for an entry beginning with: {0}</resource>
 
 <resource key="EXC_PPR_ERROR_PREFIX">Server Exception during PPR, #{0}</resource>
+
+<resource key="STYLE_ENTRY_RETRIEVAL_INTERRUPTED">Retrieval of style sheet {0} was interrupted.  Will retry on next request</resource>
+
+<resource key="STYLE_ENTRY_CREATION_FAILED">Creation of style sheet {0} has failed.  Will retry on next request.</resource>
+
 </resources>

-- 
To stop receiving notification emails like this one, please contact
"commits@myfaces.apache.org" <co...@myfaces.apache.org>.