Seam Remoting - Marshalling bug

[pgmjsd]

I think we've just found a subtle multi-threading issue in the Seam Remoting marshalling code.   The issue only happens when lots of threads are doing Seam Remoting calls.

org.jboss.seam.remoting.wrapper.DateWrapper uses a SimpleDateFormat to convert values to and from text.   However, this class has a static instance of SimpleDateFormat, which is not thread safe.   If you have lots of threads doing seem remoting (we do) then eventually the marshalling will produce weird results.

[pgmjsd]

We're getting hit pretty badly now by this bug.   The patch we uploaded to JIRA works well, and we'll be deploying it to production soon.   Maybe it could sneak into 2.1.2?  :)

https://jira.jboss.org/jira/browse/JBSEAM-4038

[shane.bryzak]

How does the performance of the ThreadLocal compare with just creating a new SimpleDateFormat instance each time?

[pgmjsd]

Shane Bryzak wrote on Apr 14, 2009 10:41:


How does the performance of the ThreadLocal compare with just creating a new SimpleDateFormat instance each time?

The difference was not measurable in some simple tests we did.   The patch uses thread local because that was more similar to the existing code.

[shane.bryzak]

I've fixed this in SVN, by using the new instance approach (my research suggested that the performance of creating a new instance vs thread local is negligible).

[pgmjsd]

Shane Bryzak wrote on Apr 14, 2009 23:39:


I've fixed this in SVN, by using the new instance approach (my research suggested that the performance of creating a new instance vs thread local is negligible).

Great!  I guess you folks have already branched for the release so it's probably too late for that.

[pgmjsd]

Shane Bryzak wrote on Apr 14, 2009 23:39:


I've fixed this in SVN, by using the new instance approach (my research suggested that the performance of creating a new instance vs thread local is negligible).

Seems like thread local can be very much faster than creating a new instance:
take a look at this