[jira] Commented: (TAPESTRY-1229) AssetService not thread safe

["Greg Woolsey (JIRA)" <ta...@jakarta.apache.org>]

    [ https://issues.apache.org/jira/browse/TAPESTRY-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12465674 ] 

Greg Woolsey commented on TAPESTRY-1229:
----------------------------------------

Here's what I did in our app, in an alternate asset service implementation class.  I just copied AssetService and changed the static DateFormat to a static final ObjectPool from Commons-Pool.  Specifically, I used a StackObjectPool, as it is simple and thread-safe:

private static final ObjectPool CACHED_FORMAT_POOL = new StackObjectPool(new PoolableDateFormatFactory(CACHED_FORMAT, CACHED_FORMAT_LOCALE));

the PoolableObjectFactory used is just a simple class extending BasePoolableObjectFactory and taking the SimpleDateFormat constructor parameters as it's constructor parameters.  It then returns new SimpleDateFormat instances as requested in makeObject().

The changes to AssetService.cachedResource() are only in the IF block for the modified-since header:

	  if (header != null) {
		  DateFormat format = null;
			try {
				format = (DateFormat) CACHED_FORMAT_POOL.borrowObject();
				modify = format.parse(header).getTime();
			} catch (ParseException e) { 
				e.printStackTrace(); // original catch block
			} catch (Exception e) {
				throw new RuntimeException("error getting pooled date format", e);
			} finally {
				if (format != null) {
					try {
						CACHED_FORMAT_POOL.returnObject(format);
					} catch (Exception e) {
						// ignore - no tear-down needed for returned objects, and the pool will just create a new one if it needs to.
					}
				}
			}
		}
 

> AssetService not thread safe
> ----------------------------
>
>                 Key: TAPESTRY-1229
>                 URL: https://issues.apache.org/jira/browse/TAPESTRY-1229
>             Project: Tapestry
>          Issue Type: Bug
>          Components: Core, Framework
>    Affects Versions: 4.1.1
>            Reporter: Greg Woolsey
>            Priority: Blocker
>
> AssetService declares this field:
>     static final DateFormat CACHED_FORMAT = new SimpleDateFormat("EEE, d MMM yyyy HH:mm:ss Z", Locale.ENGLISH);
> However, DateFormat objects are explicitly NOT thread safe.  We've encountered this in our in-house concurrency testing.  We'll have to patch our local 4.1.1 to fix this before deploying our application.
> a quick search did not find any other references to SimpleDateFormat that look like they could have the same issue.
> A simple CommonsPool object pool of date format objects would fix the problem.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org