Re: Babysitting ThreadLocals
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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
-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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
-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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
-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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Konstantin, On 11/23/11 1:21 PM, Konstantin Kolinko wrote: > 2011/11/23 Christopher Schultz : >> On 11/23/11 11:29 AM, Caldarale, Charles R wrote: >>>> From: Christopher Schultz >>>> [mailto:ch...@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 -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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
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 dayFormat = new > ThreadLocal() { >public SimpleDateFormat initialValue() >{ >return new SimpleDateFormat("-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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
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 dayFormat = new ThreadLocal() { public SimpleDateFormat initialValue() { return new SimpleDateFormat("-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 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Babysitting ThreadLocals
> From: Chema [mailto:demablo...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
2011/11/23 Christopher Schultz : > On 11/23/11 11:29 AM, Caldarale, Charles R wrote: >>> From: Christopher Schultz [mailto:ch...@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 -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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
>> 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
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" < ch...@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-unsubscr...@tomcat.apache.org > For additional commands, e-mail: users-h...@tomcat.apache.org > >
Re: Babysitting ThreadLocals
-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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
> > 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
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Chuck, On 11/23/11 11:29 AM, Caldarale, Charles R wrote: >> From: Christopher Schultz [mailto:ch...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
-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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Babysitting ThreadLocals
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
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 : > -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 dayFormat = new > ThreadLocal() { > public SimpleDateFormat initialValue() > { > return new SimpleDateFormat("-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 > 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-unsubscr...@tomcat.apache.org > For additional commands, e-mail: users-h...@tomcat.apache.org > > - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Babysitting ThreadLocals
> From: Christopher Schultz [mailto:ch...@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.
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 dayFormat = new ThreadLocal() { public SimpleDateFormat initialValue() { return new SimpleDateFormat("-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 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org