myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andy Schwartz (JIRA)" <>
Subject [jira] [Commented] (TRINIDAD-2463) Concurrency issues in CachingResourceLoader
Date Wed, 26 Mar 2014 00:46:15 GMT


Andy Schwartz commented on TRINIDAD-2463:

The attached patch addresses this:

> 1. Make CachingResourceLoader.URLStreamHandlerImpl thread safe. 

By storing URLStreamHandlerImpl's mutable state in a new immutable CachedContents class that
is managed via an AtomicReference.

And this:

> 2. Make CachingResourceLoader more tolerant of content length changes.

By adding checks for content length vs. cached content size mismatches.  In the event that
a mismatch is detected, we a) clear our cached data, which should allow the cache to recover
on subsequent requests and b) send an error back to the browser, which seems like an improvement
over sending the potentially bogus contents.

> Concurrency issues in CachingResourceLoader
> -------------------------------------------
>                 Key: TRINIDAD-2463
>                 URL:
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>    Affects Versions: 2.1.0-core
>            Reporter: Andy Schwartz
>            Assignee: Andy Schwartz
>            Priority: Minor
>         Attachments: trinidad-2463.patch
> I am seeing intermittent failures when serving up skin-generated .css files via the Trinidad
ResourceServlet.  When the failure occurs, the ResourceServlet sends a response with conflicting
information: the response specifies a Content-Length header that does not match the number
of bytes of data that are sent.  In particular, the Content-Length header specifies the correct
size of the .css file as it appears on the file system, but the content that is sent back
to the browser is truncated.
> Although I haven’t been able to reproduce the problem in a debugger, I suspect that
the problem is due to the unsafe way in which CachingResourceLoader.URLStreamHandlerImpl deals
with withs cached state.
> One obvious issue with this code is that it is not thread safe.  It performs non-atomic
operations (check and set) of the _contents and _contentsModified fields without synchronization
(or without any other scheme to ensure thread safety).  Additionally, there is nothing protecting
against these fields being modified concurrently.  Also, there is no attempt to ensure that
the values assigned to these fields are published safely.
> This is bad.
> Another possible concern is that in theory we could end up reading the .css file contents
off of the file system while this file is being written to by a second thread.  In this case,
our cached contents may not reflect the full contents of the file as it (eventually) appears
on the file system.   However, since we always retrieve the value for the Content-Length response
header from the file system, we always send the latest file size, even if this does not match
the number of bytes that we have cached.
> This could explain the mismatch that I am seeing between the Content-Length header and
the size of our response payload.
> We need to do two things:
> 1.  Make CachingResourceLoader.URLStreamHandlerImpl thread safe.
> 2.  Make CachingResourceLoader more tolerant of content length changes.

This message was sent by Atlassian JIRA

View raw message