You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Christopher Schultz <ch...@christopherschultz.net> on 2011/11/23 16:48:16 UTC
Babysitting ThreadLocals
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
All,
I've got a servlet that needs to log every request (potentially big
requests) to files on the disk. In order to do that in a
reasonably-tidy way, we write each file into a directory with the
current date in the path, something like this:
.../logs/2011-11-23/request-XYX.log
To do this, we have a SimpleDateFormat object that we use to ensure we
target the right directory. Since SimpleDateFormat isn't threadsafe,
we have two choices: synchronize or use ThreadLocal. We have opted for
the latter: ThreadLocal.
Our servlet defines the ThreadLocal to be protected (because this is a
base class for several servlets that all do similar things) and
transient (because we just don't need it to be serialized) and
override the initialValue method, like this:
protected transient ThreadLocal<SimpleDateFormat> dayFormat = new
ThreadLocal<SimpleDateFormat>() {
public SimpleDateFormat initialValue()
{
return new SimpleDateFormat("yyyy-MM-dd");
}
};
In the servlet's destroy method, we dutifully call dayFormat.remove().
Tomcat complains that we are leaving sloppy ThreadLocals around on
shutdown. Duh: Servlet.destroy is called by a single thread and won't
actually remove the ThreadLocal in any meaningful way.
So, my question is whether or not there is a good way to clean-out the
ThreadLocals from our webapp?
Given the declaration above, we are creating a new class which will be
loaded by our webapp's ClassLoader and therefore pinning that
ClassLoader in memory definitely causing a memory leak across reploy
cycles.
One way to avoid this would be to have a library at the server-level
that only contains this simple ThreadLocat<SimpleDateFormat>
definition, but that seems like kind of an awkward solution.
Removing the ThreadLocal after every request of course means that the
use of ThreadLocal is entirely useless.
Should I stop worrying about the overhead of creating a
SimpleDateFormat? Should I look for a threadsafe implementation of
SimpleDateFormat (maybe in commons-lang or something)? Should I
synchronize access to the object?
Any suggestions would be very helpful.
Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7NFcAACgkQ9CaO5/Lv0PDIoACgrc5nNYGXUxjJ+hz1kWpiIL6J
SpYAoJQ6dcxCi4WmPX+1BJs9b3c+UQB5
=3bj2
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
RE: Babysitting ThreadLocals
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Chema [mailto:demablogia@gmail.com]
> Subject: Re: Babysitting ThreadLocals
> Do you mean that read operations (getters) in not-threadsafe objects
> are not an atomic operations and could retrieve "dirty" values cause
> sharing across threads?
Correct. Not-thread-safe means just what it sounds like.
> So, singleton objects must be threadsafe to be a rea singleton ?
Depends on the object. If you have written the class code to insure that ostensibly read-only operations do not mutate the object in any way, then you only need to provide synchronization when there's a risk of a non-read-only operation being active. If you didn't write the code, you have no guarantee that non-thread-safe getter methods don't mutate the object internally during their processing.
- Chuck
THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Chema,
On 11/23/11 1:10 PM, Chema wrote:
>>> The string of the date format is constant. However the
>>> SimpleDateFormat
>> class is not threadsafe, so you will hit intermittant issues when
>> sharing across threads
>
> Do you mean that read operations (getters) in not-threadsafe
> objects are not an atomic operations and could retrieve "dirty"
> values cause sharing across threads?
As Chuck says, that depends upon the class.
In the case of SimpleDateFormat, there is a Calendar object used
internally with no synchronization, so multiple threads cannot safely
use java.text.SimpleDateFormat without fear of mass confusion.
If you didn't know that, now you do: don't use a shared
SimpleDateFormat in a threaded environment without any kind of protection.
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7PqpQACgkQ9CaO5/Lv0PDragCgrluaNuJ1Xs3tMGvpHauEts7d
VhYAn1vyKtmd/pT1FGzbibXJwlGfvI56
=oo7Q
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Chema <de...@gmail.com>.
>> The string of the date format is constant. However the SimpleDateFormat
> class is not threadsafe, so you will hit intermittant issues when sharing
> across threads
Do you mean that read operations (getters) in not-threadsafe objects
are not an atomic operations and could retrieve "dirty" values cause
sharing
across threads?
So, singleton objects must be threadsafe to be a rea singleton ?
Maybe my doubts are very basic but I didn't know about these issues ...
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Filippo Machi <fi...@gmail.com>.
Ciao Christopher, i heard Joda has a thread safe date
parser/fotmatter..remember to check it doesn't use threadlocals too :)
Hth
Fil
Il giorno 23/nov/2011 17.57, "Christopher Schultz" <
chris@christopherschultz.net> ha scritto:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Chris,
>
> On 11/23/11 11:46 AM, chris derham wrote:
> > If you do this, and fine that creating these objects is taking more
> > time, then perhaps one method would be to use a weak object
> > reference to the thread local. That way you would get the best of
> > both worlds - no memory leak and reduced creation of
> > SimpleDateFormat.
>
> I hadn't thought of using a WeakReference. I wonder how often the GC
> would kill the reference between requests, though. We only get one
> maybe every 10 seconds or so right now, so it's possible that we'd
> have the memory churn associated with creating a new object for every
> request anyway.
>
> > However most people coding probably won't know what a ThreadLocal
> > class is/does, let alone a Weak memory reference. IMO it would be
> > easier just to code the easy way
>
> Yeah, this is definitely over-engineered at this point, especially
> given that it's not actually working the way it should (that is, we've
> got a memory leak).
>
> I think I'll look into the commons-lang date formatter to see if
> there's any reason to use it instead of SimpleDateFormat. If it
> performs reasonably under load (that is, doesn't have much in the way
> of synchronization and creates fewer objects than "new
> SimpleDateFormat") then I'll probably go with that... we already have
> a dependency on that library, anyway.
>
> Thanks,
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk7NJd4ACgkQ9CaO5/Lv0PBcwQCfaZ3OcDMwkgXRc6HIkNMF2ddM
> oHcAoLqaYghNBDFm3zIMS2mJSneRo3Fa
> =yw3K
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Chris,
On 11/23/11 11:46 AM, chris derham wrote:
> If you do this, and fine that creating these objects is taking more
> time, then perhaps one method would be to use a weak object
> reference to the thread local. That way you would get the best of
> both worlds - no memory leak and reduced creation of
> SimpleDateFormat.
I hadn't thought of using a WeakReference. I wonder how often the GC
would kill the reference between requests, though. We only get one
maybe every 10 seconds or so right now, so it's possible that we'd
have the memory churn associated with creating a new object for every
request anyway.
> However most people coding probably won't know what a ThreadLocal
> class is/does, let alone a Weak memory reference. IMO it would be
> easier just to code the easy way
Yeah, this is definitely over-engineered at this point, especially
given that it's not actually working the way it should (that is, we've
got a memory leak).
I think I'll look into the commons-lang date formatter to see if
there's any reason to use it instead of SimpleDateFormat. If it
performs reasonably under load (that is, doesn't have much in the way
of synchronization and creates fewer objects than "new
SimpleDateFormat") then I'll probably go with that... we already have
a dependency on that library, anyway.
Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7NJd4ACgkQ9CaO5/Lv0PBcwQCfaZ3OcDMwkgXRc6HIkNMF2ddM
oHcAoLqaYghNBDFm3zIMS2mJSneRo3Fa
=yw3K
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by chris derham <ch...@derham.me.uk>.
>
> A silly question:
>
> why do you use a ThreadLocal to store a constant value for entire
> application? why not a static variable or store into web application
> context , by example ?
>
> The string of the date format is constant. However the SimpleDateFormat
class is not threadsafe, so you will hit intermittant issues when sharing
across threads.
> So, my question is whether or not there is a good way to clean-out the
> > ThreadLocals from our webapp?
>
> It would be much simpler code to read/write/maintain if you just create
new ones each time - as Charles says. Then profile the app, and only if the
creation of simpleDateFormat objects is slowing the app, then try to
optimise.
If you do this, and fine that creating these objects is taking more time,
then perhaps one method would be to use a weak object reference to the
thread local. That way you would get the best of both worlds - no memory
leak and reduced creation of SimpleDateFormat. However most people coding
probably won't know what a ThreadLocal class is/does, let alone a Weak
memory reference. IMO it would be easier just to code the easy way
Chris
Re: Babysitting ThreadLocals
Posted by Daniel Mikusa <dm...@vmware.com>.
On Wed, 2011-11-23 at 07:48 -0800, Christopher Schultz wrote:
> Should I look for a threadsafe implementation of
> SimpleDateFormat (maybe in commons-lang or something)?
I haven't used this, but it seems to be a drop in replacement for
SimpleDateFormat.
https://commons.apache.org/lang/api-2.5/org/apache/commons/lang/time/FastDateFormat.html
Dan
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Chema,
On 11/23/11 11:31 AM, Chema wrote:
> A silly question:
>
> why do you use a ThreadLocal to store a constant value for entire
> application? why not a static variable or store into web
> application context , by example ?
It's not a silly question in general, but I did specifically mention
that SimpleDateFormat is not threadsafe. Therefore, I cannot use a
constant value for the entire application.
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7NI00ACgkQ9CaO5/Lv0PAYhwCgk05bTrh/cg8hBQKOecah/q8n
7NMAoKFGB7yKDc1afLT6wxt8/Y+N7l5Z
=pY5r
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Chema <de...@gmail.com>.
A silly question:
why do you use a ThreadLocal to store a constant value for entire
application? why not a static variable or store into web application
context , by example ?
Thanks
2011/11/23 Christopher Schultz <ch...@christopherschultz.net>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> All,
>
> I've got a servlet that needs to log every request (potentially big
> requests) to files on the disk. In order to do that in a
> reasonably-tidy way, we write each file into a directory with the
> current date in the path, something like this:
>
> .../logs/2011-11-23/request-XYX.log
>
> To do this, we have a SimpleDateFormat object that we use to ensure we
> target the right directory. Since SimpleDateFormat isn't threadsafe,
> we have two choices: synchronize or use ThreadLocal. We have opted for
> the latter: ThreadLocal.
>
> Our servlet defines the ThreadLocal to be protected (because this is a
> base class for several servlets that all do similar things) and
> transient (because we just don't need it to be serialized) and
> override the initialValue method, like this:
>
> protected transient ThreadLocal<SimpleDateFormat> dayFormat = new
> ThreadLocal<SimpleDateFormat>() {
> public SimpleDateFormat initialValue()
> {
> return new SimpleDateFormat("yyyy-MM-dd");
> }
> };
>
> In the servlet's destroy method, we dutifully call dayFormat.remove().
> Tomcat complains that we are leaving sloppy ThreadLocals around on
> shutdown. Duh: Servlet.destroy is called by a single thread and won't
> actually remove the ThreadLocal in any meaningful way.
>
> So, my question is whether or not there is a good way to clean-out the
> ThreadLocals from our webapp?
>
> Given the declaration above, we are creating a new class which will be
> loaded by our webapp's ClassLoader and therefore pinning that
> ClassLoader in memory definitely causing a memory leak across reploy
> cycles.
>
> One way to avoid this would be to have a library at the server-level
> that only contains this simple ThreadLocat<SimpleDateFormat>
> definition, but that seems like kind of an awkward solution.
>
> Removing the ThreadLocal after every request of course means that the
> use of ThreadLocal is entirely useless.
>
> Should I stop worrying about the overhead of creating a
> SimpleDateFormat? Should I look for a threadsafe implementation of
> SimpleDateFormat (maybe in commons-lang or something)? Should I
> synchronize access to the object?
>
> Any suggestions would be very helpful.
>
> Thanks,
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk7NFcAACgkQ9CaO5/Lv0PDIoACgrc5nNYGXUxjJ+hz1kWpiIL6J
> SpYAoJQ6dcxCi4WmPX+1BJs9b3c+UQB5
> =3bj2
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Sylvain Laurent <sl...@apache.org>.
On 25 nov. 2011, at 15:58, Christopher Schultz wrote:
> On 11/24/11 4:02 PM, Sylvain Laurent wrote:
>> I don't think this ThreadLocal creates a real leak of classloader.
>> It would if dayFormat was static.
>
> IIRC, ThreadLocal essentially puts a key/value pair in a Map in the
> Thread. I dunno what kind of reference it is, but I suspect it's a
> normal, strong reference. That means that the Thread itself retains a
> reference to the instance of the inner class in my servlet. That's
> just not going to become available for collection anytime soon.
Actually, in Sun's implementation (1.5 and 1.6 at least), ThreadLocal are implemented with a kind of WeakHashMap in a instance variable of Thread, using your ThreadLocal instance as a weak key and the actual value you stored as value with a strong reference.
In your example, the reference to the ThreadLocal instance is stored in an instance variable of your Servlet. So, when your app is stopped, tomcat releases its reference to your Servlet instance so that it can be collected and your ThreadLocal instance too.
Since in your case the value that is bound in the ThreadLocal for each thread is a JRE class (SimpleDateFormat), it does not reference the webapp classloader. The latter can then be collected (provided there are no other references pinning it in memory).
Note that I was wrong when I wrote that there would be a leak if dayFormat was static : that would only be the case if the value bound in the ThreadLocal was an instance of a class that is loaded by the webapp. It's not the case here (SimpleDateFormat), so that even with a static dayFormat, the classloader would be GCed.
Sylvain
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Sylvain,
On 11/24/11 4:02 PM, Sylvain Laurent wrote:
> I don't think this ThreadLocal creates a real leak of classloader.
> It would if dayFormat was static.
IIRC, ThreadLocal essentially puts a key/value pair in a Map in the
Thread. I dunno what kind of reference it is, but I suspect it's a
normal, strong reference. That means that the Thread itself retains a
reference to the instance of the inner class in my servlet. That's
just not going to become available for collection anytime soon.
> But you may still see warnings issued by tomcat when the
> application is stopped because of this problem
> http://wiki.apache.org/tomcat/MemoryLeakProtection#threadLocalPseudoLeak
>
>
After some time and if all the threads of the server are sollicited
> sufficiently, the classloader will be eventually collected.
I must admit that I haven't instrumented the VM after a redeploy to
check to see if the ThreadLocals are eventually restarted.
> With tomcat 7, there's no leak since threads are renewed, but you
> might still see the warnings.
I am using Tomcat 7 -- I had forgotten about that feature. So, I think
you're right: the Threads will eventually be trashed and the
WebappClassLoader will be discarded. Thanks for pointing that out.
> IMHO, you'd rather either stop worrying and recreate a new
> SimpleDateFormat, unless actual tests show a real bottleneck. In
> that case, go with another implementation like FastDateFormat. It
> will be much cleaner than playing with ThreadLocals...
Absolutely.
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7PrR8ACgkQ9CaO5/Lv0PC3EQCfTBNGMNCl0Nk882pDrrHMnVWH
3+oAoLbdnk0FgHs907hWSzq+5PsAyASl
=mB/F
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Sylvain Laurent <sl...@apache.org>.
On 23 nov. 2011, at 16:48, Christopher Schultz wrote:
> Our servlet defines the ThreadLocal to be protected (because this is a
> base class for several servlets that all do similar things) and
> transient (because we just don't need it to be serialized) and
> override the initialValue method, like this:
>
> protected transient ThreadLocal<SimpleDateFormat> dayFormat = new
> ThreadLocal<SimpleDateFormat>() {
> public SimpleDateFormat initialValue()
> {
> return new SimpleDateFormat("yyyy-MM-dd");
> }
> };
>
> In the servlet's destroy method, we dutifully call dayFormat.remove().
> Tomcat complains that we are leaving sloppy ThreadLocals around on
> shutdown. Duh: Servlet.destroy is called by a single thread and won't
> actually remove the ThreadLocal in any meaningful way.
> So, my question is whether or not there is a good way to clean-out the
> ThreadLocals from our webapp?
>
> Given the declaration above, we are creating a new class which will be
> loaded by our webapp's ClassLoader and therefore pinning that
> ClassLoader in memory definitely causing a memory leak across reploy
> cycles.
I don't think this ThreadLocal creates a real leak of classloader. It would if dayFormat was static.
But you may still see warnings issued by tomcat when the application is stopped because of this problem http://wiki.apache.org/tomcat/MemoryLeakProtection#threadLocalPseudoLeak
After some time and if all the threads of the server are sollicited sufficiently, the classloader will be eventually collected.
With tomcat 7, there's no leak since threads are renewed, but you might still see the warnings.
IMHO, you'd rather either stop worrying and recreate a new SimpleDateFormat, unless actual tests show a real bottleneck. In that case, go with another implementation like FastDateFormat. It will be much cleaner than playing with ThreadLocals...
Sylvain
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Konstantin,
On 11/23/11 1:21 PM, Konstantin Kolinko wrote:
> 2011/11/23 Christopher Schultz <ch...@christopherschultz.net>:
>> On 11/23/11 11:29 AM, Caldarale, Charles R wrote:
>>>> From: Christopher Schultz
>>>> [mailto:chris@christopherschultz.net] Subject: Babysitting
>>>> ThreadLocals
>>>
>>>> Removing the ThreadLocal after every request of course means
>>>> that the use of ThreadLocal is entirely useless.
>>>
>>>> Should I stop worrying about the overhead of creating a
>>>> SimpleDateFormat?
>>>
>>> Given that the cost of generating and writing a log entry is
>>> going to vastly outweigh any object creation or synchronization
>>> impact, then, yes, you should stop worrying.
>>
>> External reality checks are always useful. ;)
>
> The yyyy-MM-dd value changes only ~365 times a year. You do not
> need to regenerate it every second.
Correct. This is only the code for "protecting" the SimpleDateFormat.
> Tomcat does some clever things when it needs to generate timestamp
> for logging purposes (e.g. in org.apache.juli.OneLineFormatter),
> but that looks like an overkill for your use case.
I actually got the idea of using ThreadLocal from Tomcat's logging
code. Tomcat has the distinct advantage of being loaded at a higher
ClassLoader level and therefore won't leak its own ThreadLocals across
webapp restarts :)
I think most of this will be overkill, at least for the minimal load
we're getting as of now. If we were talking more than maybe 50
requests per second I might start looking at ways to reduce memory and
CPU overhead for this kind of thing. But Chuck is right: the disk is
the bottleneck, here, and probably always will be until we move to a
more message-oriented logging scheme (where the disk on the other end
will be the bottleneck) :)
Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7PqfwACgkQ9CaO5/Lv0PA4iACeIKmdDmj5mr3yORb+h0+G2LDy
Tz8An0R13Akuc1NnxHfuvfWU24G1i5g+
=EvD/
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/23 Christopher Schultz <ch...@christopherschultz.net>:
> On 11/23/11 11:29 AM, Caldarale, Charles R wrote:
>>> From: Christopher Schultz [mailto:chris@christopherschultz.net]
>>> Subject: Babysitting ThreadLocals
>>
>>> Removing the ThreadLocal after every request of course means that
>>> the use of ThreadLocal is entirely useless.
>>
>>> Should I stop worrying about the overhead of creating a
>>> SimpleDateFormat?
>>
>> Given that the cost of generating and writing a log entry is going
>> to vastly outweigh any object creation or synchronization impact,
>> then, yes, you should stop worrying.
>
> External reality checks are always useful. ;)
The yyyy-MM-dd value changes only ~365 times a year. You do not need
to regenerate it every second.
Tomcat does some clever things when it needs to generate timestamp for
logging purposes (e.g. in org.apache.juli.OneLineFormatter), but that
looks like an overkill for your use case.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Chuck,
On 11/23/11 11:29 AM, Caldarale, Charles R wrote:
>> From: Christopher Schultz [mailto:chris@christopherschultz.net]
>> Subject: Babysitting ThreadLocals
>
>> Removing the ThreadLocal after every request of course means that
>> the use of ThreadLocal is entirely useless.
>
>> Should I stop worrying about the overhead of creating a
>> SimpleDateFormat?
>
> Given that the cost of generating and writing a log entry is going
> to vastly outweigh any object creation or synchronization impact,
> then, yes, you should stop worrying.
External reality checks are always useful. ;)
Thanks,
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7NI3MACgkQ9CaO5/Lv0PA0SwCgo3kT2d2I0QoWGpPE3cl3C7It
9isAniS9prBskorh9J5dDxGrutjKXCla
=/h/K
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
RE: Babysitting ThreadLocals
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Christopher Schultz [mailto:chris@christopherschultz.net]
> Subject: Babysitting ThreadLocals
> Removing the ThreadLocal after every request of course means
> that the use of ThreadLocal is entirely useless.
> Should I stop worrying about the overhead of creating a
> SimpleDateFormat?
Given that the cost of generating and writing a log entry is going to vastly outweigh any object creation or synchronization impact, then, yes, you should stop worrying.
- Chuck
THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
Re: Babysitting ThreadLocals
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Terrence,
On 11/23/11 8:13 PM, Terence M. Bandoian wrote:
> Adding Thread.yield() eliminated the error message from the log.
No, this is a legitimate leak.
In order to fix it, I'd have to clean all the threads in the thread
pool (because it's a ThreadLocal). It's just not worth it.
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk7PqwMACgkQ9CaO5/Lv0PDANwCfX4o1w2wuAvBdhXauOITzJu/l
/gsAn3IMsAG96X1lCIgbxGzYQxhDGLnl
=gGW6
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Babysitting ThreadLocals
Posted by "Terence M. Bandoian" <te...@tmbsw.com>.
On 1:59 PM, Christopher Schultz wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> All,
>
> I've got a servlet that needs to log every request (potentially big
> requests) to files on the disk. In order to do that in a
> reasonably-tidy way, we write each file into a directory with the
> current date in the path, something like this:
>
> .../logs/2011-11-23/request-XYX.log
>
> To do this, we have a SimpleDateFormat object that we use to ensure we
> target the right directory. Since SimpleDateFormat isn't threadsafe,
> we have two choices: synchronize or use ThreadLocal. We have opted for
> the latter: ThreadLocal.
>
> Our servlet defines the ThreadLocal to be protected (because this is a
> base class for several servlets that all do similar things) and
> transient (because we just don't need it to be serialized) and
> override the initialValue method, like this:
>
> protected transient ThreadLocal<SimpleDateFormat> dayFormat = new
> ThreadLocal<SimpleDateFormat>() {
> public SimpleDateFormat initialValue()
> {
> return new SimpleDateFormat("yyyy-MM-dd");
> }
> };
>
> In the servlet's destroy method, we dutifully call dayFormat.remove().
> Tomcat complains that we are leaving sloppy ThreadLocals around on
> shutdown. Duh: Servlet.destroy is called by a single thread and won't
> actually remove the ThreadLocal in any meaningful way.
>
> So, my question is whether or not there is a good way to clean-out the
> ThreadLocals from our webapp?
>
> Given the declaration above, we are creating a new class which will be
> loaded by our webapp's ClassLoader and therefore pinning that
> ClassLoader in memory definitely causing a memory leak across reploy
> cycles.
>
> One way to avoid this would be to have a library at the server-level
> that only contains this simple ThreadLocat<SimpleDateFormat>
> definition, but that seems like kind of an awkward solution.
>
> Removing the ThreadLocal after every request of course means that the
> use of ThreadLocal is entirely useless.
>
> Should I stop worrying about the overhead of creating a
> SimpleDateFormat? Should I look for a threadsafe implementation of
> SimpleDateFormat (maybe in commons-lang or something)? Should I
> synchronize access to the object?
>
> Any suggestions would be very helpful.
>
> Thanks,
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk7NFcAACgkQ9CaO5/Lv0PDIoACgrc5nNYGXUxjJ+hz1kWpiIL6J
> SpYAoJQ6dcxCi4WmPX+1BJs9b3c+UQB5
> =3bj2
> -----END PGP SIGNATURE-----
Hi, Chris-
This sounds very similar to the problem I faced when trying to terminate
an executor in contextDestroyed. I worked around that by calling
Thread.yield() after terminating the executor.
public void contextDestroyed( ServletContextEvent sce )
{
if ( executor != null )
{
boolean isTerminated = false;
executor.shutdown();
do
{
try
{
isTerminated = executor.awaitTermination(
1, TimeUnit.SECONDS );
}
catch ( InterruptedException ignore )
{
}
}
while ( !isTerminated );
executor = null;
Thread.yield();
}
}
Adding Thread.yield() eliminated the error message from the log.
Hope that helps.
-Terence Bandoian
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org