Never use synchronized methods or lock on this
Especially when extending a 3rd party base class.
This is a known best practice, but when I read about it I natrually assumed I was smarter than the author of the best practice. The reason not to use synchronized methods (or lock(this)) is that other code might lock on your object too, thus causing nasty deadlocks.
I figured this wouldn’t happen because ‘who would just lock on my object, there’s no chance of that’. Well, this is obviously not safe, but especially so when extending a 3rd-party base class. In my case, I was extending log4j’s AppenderSkeleton, and found out the hard way that log4j obtains locks on the appenders.
The solution:
- Use a private lock object (duh), seperating your intended lock semantics from whatever evil outside code will use
- Stop assuming that I know best and ‘it will never happen’
