On 07/09/10 12:10, Michael McMahon wrote:
Chris Hegarty wrote:


On 07/09/10 11:27, Michael McMahon wrote:
Chris Hegarty wrote:


On 07/09/10 10:10, Michael McMahon wrote:
Chris Hegarty wrote:
Michael,

CR 6967684: httpserver using a non thread-safe SimpleDateFormat

Bug description says it all. I guess in reality this may have gone
unseen for so long because, if two or more threads were servicing
requests the dates they would be generating should be fairly similar,
but worst case they could actually see partial of corrupt dates. This
bug causes random httpserver tests to fail when run with assertions
enabled.

The most reasonable solution is to use a thread-local instance of the
formatter, rather than creating one per request.

Webrev:
http://cr.openjdk.java.net/~chegar/6967684/webrev.00/webrev/

Thanks,
-Chris.
Interesting bug.

I wonder would it be better to use the ServerTimerTask to read the
time
of day each time
it wakes up and the thread handlers would just read this value each
time
they need it?

To clarify, the ServerTimerTask would provide the already formatted
date String, right?

yes.
That's probably what should have been done originally, as it scales
better with more traffic
and formatting dates is probably quite compute intensive.

A simple test shows 1,000,000 formats of new Date() takes 4407 millis,
I don't think this is too expensive.

Does that include the cost of the new Date() itself? in the
ServerTimerTask we're already calling
System.currentTimeMillis() and that could be used to create the Date,
avoid another call to get the time
from the OS

The test includes the 'new Date()'. I don't think
System.currentTimeMillis() is really that expensive. It effectively
calls gettimeofday, which is a system call but typically is quick
since it just reads a register value and doesn't need to call into the
kernel.

If ServerTimerTask is to provide the date then we'll have to
synchronize its access. This may actually be more expensive for
servers with many many threads. It would be a highly contended monitor
since every thread will be required to hold it, albeit not for long.

I'm not sure it would need to be synchronized. Any thread would get a
valid reference to either the new
value or the old one. It wouldn't really matter.

It's not just the old ref. You could get corrupted data, and are never
guaranteed to every see a more recent version. Not using
synchronization here would potentially be worse than the original bug.

What data could be corrupted?

I'm pretty sure object references can't be corrupted. Callers will see
either a valid reference
to the old string or a valid reference to the new string - the string
being the fully formatted
date.

It would really depend on how it is implemented, but my other point stands. Another thread will never be guaranteed to see an updated date string. This goes beyond the previously generated one, theoretically a worker thread may get the date once and never see any update.

-Chris.


- Michael.

Reply via email to