[
https://issues.apache.org/jira/browse/TRINIDAD-2463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13947400#comment-13947400
]
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: https://issues.apache.org/jira/browse/TRINIDAD-2463
> 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
(v6.2#6252)
