Raible Designs

> The other bug is "Inconsistent synchronization" in
> UserCounterListener.contextInitialized() method.
> Any tips on solving this one are appreciated.

I guess it's because the methods contextInitialized & contextDestroyed are not synchronized and they use "users" and "counter", while other synchronized methods also use them.

Posted by Guillaume Poirier on August 19, 2004 at 04:15 PM MDT #

This article might help: http://www-106.ibm.com/developerworks/library/j-jtp06294.html?Open&ca=daw-ja-news Specifically the "Being nice to the memory model" section: In "Fixing the Java Memory Model, Part 1," I reviewed the basic rule for synchronization -- whenever reading a variable that might have last been written by another thread, or writing a variable that might next be read by another thread, you must synchronize. It is easy to "forget" this rule, especially when reading -- but doing so creates numerous risks for the thread-safety of your program. This sort of bug is commonly introduced when maintaining a class that was originally properly synchronized, but the thread-safety requirements were not fully understood by the maintainer. "Fortunately, FindBugs has a number of detectors that can help identify incorrectly synchronized classes. The Inconsistent Synchronization detector is probably the most complicated detector used by FindBugs; it has to analyze the whole program, not just individual methods, use dataflow analysis to determine when a lock is held, and use heuristics to infer that a class intends to provide thread-safety guarantees. Basically, for each field, it looks at the pattern of accesses for that field, and if the majority of accesses are done with synchronization, accesses without synchronization are flagged as potential errors. Similarly, the Inconsistent Synchronization detector will issue a warning if the setter for a property is synchronized and the getter is not. In addition to inconsistent synchronization, a number of other detectors for common threading errors are included, such as waiting on a monitor with two locks held (which, while not necessarily a bug, could cause a deadlock), using the double-checked locking idiom, incorrect lazy initialization of nonvolatile fields, invoking run() on a thread instead of starting it, invoking Thread.start() from a constructor, or calling wait() without wrapping it in a loop. "

Posted by dsuspense on August 19, 2004 at 05:12 PM MDT #

I thought for sure I tried adding "public synchronized void" to those methods last night and it didn't work. Oh well, musta been late - works now! Thanks guys.

Posted by Matt Raible on August 19, 2004 at 05:57 PM MDT #

More than anything, I think if your application works and all your tests pass - then that's the best indicator of quality code. However, these tools do help and show you things that you might not realize. Thanks for the PMD reminder, that's integrated into AppFuse and I just cleaned up a couple more classes using it.

Posted by Matt Raible on August 19, 2004 at 07:27 PM MDT #

Hey Matt, Eclipse 3.1 M1 has some new quick fixes for serial ids: "Two new Java quick fixes have been added to generate serial version IDs for classes implementing java.io.Serializable." You can find out more about it in the new and noteworthy page.

Posted by Vinu on August 19, 2004 at 08:59 PM MDT #

Oops, that should've read private static final long serialVersionUID = any-long-value-here;

Posted by Santa Claus on August 20, 2004 at 07:07 AM MDT #

Actually, serialver.exe does not generate a random number, it will always generate the same number for two classes that match the criterias used by their algo. Those criterias are things like the class name, variables names and types, and the method signatures. It basically generates the same number that would be generated at runtime if none is specified in the class. What number you want there really depends on how you want to handle the possibily that an object of that class is serialized, then deserialized with a different version of the same class. If don't define any serialVersionuID, then any change to the class will have the deserialization fails, which might or might not be what you want, depending on your use case. If you set it to always the same number, then any change won't prevent the deserialization, but if your changes were not compatible, then you might get a corrupted object. But I took a quit look at AppFuse, and you don't seem to use Serliazation anywhere, do you? The only place I can think of where it would make any difference is if Tomcat serialize the content of the HttpSession. And in such case, I assume you would want to deserialization to fail when in doubt, thus not specifiying any serialVersionUID. But like I said, it depends on your particular situation.

Posted by Guillaume Poirier on August 22, 2004 at 02:56 PM MDT #

Well, like I said, I find that there are situations where it would be valid not to define any serialVersionUID, so I would question the relevance of such warning. But if you just want to get rid of the warning, and can't disable it, then I can't see any reason where the actual value would matter for you, except may be for the domain objects, if they goes in an HttpSession. That is because Tomcat, depending on your configuration, might serialize the content of the HttpSession on server shutdown, and load the session content back again on server startup. In developpement environnement, if your domain object changed, you probably want the deserialization to fail, because it could lead to unexpected exceptions. Then there's also the clustering issue, but I guess in your case you probably don't care about it.

Posted by Guillaume Poirier on August 22, 2004 at 04:14 PM MDT #