Re: Babysitting ThreadLocals

2011-11-25 Thread Christopher Schultz
-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: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

2011-11-25 Thread Christopher Schultz
-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

2011-11-25 Thread Christopher Schultz
-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

2011-11-25 Thread Christopher Schultz
-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

2011-11-25 Thread Sylvain Laurent

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

2011-11-24 Thread Sylvain Laurent

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 ThreadLocalSimpleDateFormat dayFormat = new
 ThreadLocalSimpleDateFormat() {
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

2011-11-23 Thread Caldarale, Charles R
 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.

 



Re: Babysitting ThreadLocals

2011-11-23 Thread Chema
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 ThreadLocalSimpleDateFormat dayFormat = new
 ThreadLocalSimpleDateFormat() {
        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 ThreadLocatSimpleDateFormat
 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

2011-11-23 Thread Daniel Mikusa
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

2011-11-23 Thread Christopher Schultz
-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

2011-11-23 Thread Christopher Schultz
-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

2011-11-23 Thread chris derham

 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

2011-11-23 Thread Christopher Schultz
-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

2011-11-23 Thread Filippo Machi
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

2011-11-23 Thread Chema
 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

2011-11-23 Thread Konstantin Kolinko
2011/11/23 Christopher Schultz ch...@christopherschultz.net:
 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

2011-11-23 Thread Caldarale, Charles R
 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 Thread Terence M. Bandoian

 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 ThreadLocalSimpleDateFormat  dayFormat = new
ThreadLocalSimpleDateFormat() {
 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 ThreadLocatSimpleDateFormat
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