You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2010/11/13 12:53:27 UTC
DO NOT REPLY [Bug 50265] New: FormatDateSupport.dateFormatCache lazy
init is not safe
https://issues.apache.org/bugzilla/show_bug.cgi?id=50265
Summary: FormatDateSupport.dateFormatCache lazy init is not
safe
Product: Taglibs
Version: unspecified
Platform: PC
OS/Version: Windows XP
Status: NEW
Severity: normal
Priority: P2
Component: Standard Taglib
AssignedTo: dev@tomcat.apache.org
ReportedBy: sebb@apache.org
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.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: DO NOT REPLY [Bug 50265] FormatDateSupport.dateFormatCache lazy init is not safe
Posted by Jeremy Boynes <jb...@apache.org>.
The caching change for #32311 has never been released. I'd like to suggest we revert that change and go back to creating formatters as needed (as we did in the last release). Due to the removal of synchronized on Calendar#getInstance() this will solve the original issue and not add any threading issues. If we want to improve the performance we can do that later by caching formatters in each tag instance, or by using a thread-safe date/time library uch as Joda.
Thoughts?
Jeremy
On Jan 1, 2011, at 12:18 PM, bugzilla@apache.org wrote:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=50265
>
> Jeremy Boynes <jb...@apache.org> changed:
>
> What |Removed |Added
> ----------------------------------------------------------------------------
> Depends on| |32311
>
> --- Comment #1 from Jeremy Boynes <jb...@apache.org> 2011-01-01 15:18:25 EST ---
> 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.
>
> --
> Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are the assignee for the bug.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 50265] FormatDateSupport.dateFormatCache lazy init
is not safe
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=50265
Jeremy Boynes <jb...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Depends on| |32311
--- Comment #1 from Jeremy Boynes <jb...@apache.org> 2011-01-01 15:18:25 EST ---
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.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 50265] FormatDateSupport.dateFormatCache lazy init
is not safe
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=50265
Jeremy Boynes <jb...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #2 from Jeremy Boynes <jb...@apache.org> 2011-01-02 12:35:46 EST ---
Addressed by reverting the addition of the cache added to address #32311.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org