You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2016/07/06 03:18:46 UTC
[Bug 59805] New: LocaleUtil causes memory leak
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
Bug ID: 59805
Summary: LocaleUtil causes memory leak
Product: POI
Version: 3.14-FINAL
Hardware: PC
Status: NEW
Severity: normal
Priority: P2
Component: POI Overall
Assignee: dev@poi.apache.org
Reporter: apptaro@gmail.com
org.apache.poi.util.LocaleUtil (introduced in 3.13) causes memory leak.
The following error messages are logged when redeploying an application which
uses POI 3.14:
06-Jul-2016 11:05:48.760 SEVERE
[ContainerBackgroundProcessor[StandardEngine[Catalina]]]
org.apache.catalina.loader.WebappClassLoaderBase.checkThreadLocalMapForLeaks
The web application [ROOT] created a ThreadLocal with key of type
[org.apache.poi.util.LocaleUtil$2] (value
[org.apache.poi.util.LocaleUtil$2@198b08c]) and a value of type
[java.util.Locale] (value [ja_JP]) but failed to remove it when the web
application was stopped. Threads are going to be renewed over time totry and
avoid a probable memory leak.
06-Jul-2016 11:05:48.769 SEVERE
[ContainerBackgroundProcessor[StandardEngine[Catalina]]]
org.apache.catalina.loader.WebappClassLoaderBase.checkThreadLocalMapForLeaks
The web application [ROOT] created a ThreadLocal with key of type
[org.apache.poi.util.LocaleUtil$1] (value
[org.apache.poi.util.LocaleUtil$1@14dfba9]) and a value of type
[sun.util.calendar.ZoneInfo] (value
[sun.util.calendar.ZoneInfo[id="Asia/Tokyo",offset=32400000,dstSavings=0,useDaylight=false,transitions=10,lastRule=null]])
but failed to remove it when the web application was stopped. Threads are going
to be renewed over time to try and avoid a probable memory leak.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #9 from apptaro@gmail.com ---
Looks good. Thank you very much.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #3 from apptaro@gmail.com ---
Thank you for the quick response.
Maybe you could add LocaleUtil.resetUserTimeZone and LocaleUtil.resetUserLocale
methods and call ThreadLocal.remove in them. Then it will be user's
responsibility to avoid memory leaks.
There are other ways to address the issue, such as:
- call ThreadLocal.remove for each request using a servlet filter
(Filter.doFilter)
- clear all references to the ThreadLocal when a servlet context is destroyed
(ServletContextListener.contextDestroyed)
But it seems those solutions are too much.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #7 from Javen O'Neal <on...@apache.org> ---
Are there any issues with two subsequent calls to LocaleUtil.getUserTimeZone
returning different values?
I guess this is consistent with two calls to TimeZone.getDefault()...
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #6 from apptaro@gmail.com ---
Thank you for your quick response.
Looking at the code, and I think it's better to remove initialValue from
ThreadLocal, and make getUserTimeZone/getUserLocal return default when nothing
has been set, like this:
public static TimeZone getUserTimeZone() {
TimeZone timeZone = userTimeZone.get();
return (timeZone != null) ? timeZone : TimeZone.getDefault();
}
Otherwise, users need to call resetUserTimeZone/resetUserLocale to avoid memory
leaks even when they have not called setUserTimeZone/setUserLocale.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #11 from Javen O'Neal <on...@apache.org> ---
Updated unit test to reset LocaleUtil ThreadLocals in r1751741.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
Javen O'Neal <on...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution|--- |FIXED
--- Comment #10 from Javen O'Neal <on...@apache.org> ---
Updated changelog in r1751740.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #4 from Andreas Beeker <ki...@apache.org> ---
Looking at the second response in stackoverflow - what would happen if we store
Strings in the ThreadLocal and lookup the Timezone/Locale via those Strings?
As Locale and TimeZone are classes from the root classloader, are they
connected to the web application classloader? So would it be any different than
storing Strings?
What confuses me is the term "created a ThreadLocal with key of type
[org.apache.poi.util.LocaleUtil$1]" ... so does this mean, even when storing
Strings there's always a reference to the webapp classloader? (... or is this a
week reference and doesn't count?)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #8 from Javen O'Neal <on...@apache.org> ---
(In reply to apptaro from comment #6)
> public static TimeZone getUserTimeZone() {
> TimeZone timeZone = userTimeZone.get();
> return (timeZone != null) ? timeZone : TimeZone.getDefault();
> }
Applied in r1751739.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
Javen O'Neal <on...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|INVALID |---
--- Comment #2 from Javen O'Neal <on...@apache.org> ---
(In reply to Javen O'Neal from comment #1)
> I think Catalina is reporting a false positive here.
I take that back. This may cause a PermGen memory leak since every thread holds
onto a reference to the ThreadLocal subclass.ly.
Searching for ThreadLocal memory leak, I got this result:
https://stackoverflow.com/questions/17968803/threadlocal-memory-leak
Log4j had a similar problem on their ThreadLocalMap.
This memory leak was fixed here:
https://svn.apache.org/viewvc/logging/log4j/trunk/src/main/java/org/apache/log4j/MDC.java?annotate=1231361&diff_format=f&pathrev=1231361#l177
The solution is to call ThreadLocal.remove somewhere...
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
--- Comment #5 from Javen O'Neal <on...@apache.org> ---
Made LocaleUtil a static class in r1751601
Added unit tests for LocaleUtil in r1751620 and r1751629
Added LocaleUtil#resetUserLocale and #resetUserTimeZone in r1751641
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 59805] LocaleUtil causes memory leak
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59805
Javen O'Neal <on...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |INVALID
OS| |All
Status|NEW |RESOLVED
--- Comment #1 from Javen O'Neal <on...@apache.org> ---
Thanks for the report.
I think Catalina is reporting a false positive here.
Looking over the short LocaleUtil class [1], there are two ThreadLocal
constants, but the thread local wrapping is necessary for the purpose of this
class: to globally set the locale and timezone.
The constants are static final, so the memory consumption doesn't increase with
time or usage.
The only change I would make to this class is making the class final.
I am closing this as invalid, unless someone provides a solution that I'm not
seeing. There should be no need to destroy and recreate threads due to
LocaleUtil.
[1]
https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/util/LocaleUtil.java?view=markup
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org