Bug 50265 - FormatDateSupport.dateFormatCache lazy init is not safe
Summary: FormatDateSupport.dateFormatCache lazy init is not safe
Alias: None
Product: Taglibs
Classification: Unclassified
Component: Standard Taglib (show other bugs)
Version: unspecified
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Depends on: 32311
  Show dependency tree
Reported: 2010-11-13 06:53 UTC by Sebb
Modified: 2011-01-02 12:35 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
Description Sebb 2010-11-13 06:53:23 UTC
FormatDateSupport.dateFormatCache is private and static.

It is initialised in the createFormatter method, however access is not synchronised, so the field might not be published correctly.

This can result in different threads getting different instances.
If this is acceptable, then this should be documented in the code.
Otherwise consider using IODH idiom.
Comment 1 Jeremy Boynes 2011-01-01 15:18:25 UTC
The caching was added to resolve 32311, which was opened due to contention issues with DateFormat.getTimeInstance() calling Calendar.getInstance() which was synchronized. However, the synchronized keyword was removed with Java 1.4 so this should no longer be an issue for us.

#32311 also notes that the calls to format() on the cached formatters are not thread safe and need to be synchronized; this is missing from the current implementation. However, in many applications the date/time patterns and Locale are likely to be the same and the cache only holds one instance of the formatter. By synchronizing on it we will introduce a contention point just like the the fix was trying to avoid.
Comment 2 Jeremy Boynes 2011-01-02 12:35:46 UTC
Addressed by reverting the addition of the cache added to address #32311.