Java Concurrency Pitfalls (in Scala) Answers
May 10th, 2011
Recently, Cay Horstmann posted a dozen Java concurrency pitfalls ported to Scala for extra pitfalliness. I had fun figuring them out. Here’s what I came up with.
I won’t copy all the code here, just provide my answers.
- The var
stopisn’t synchronized in any way (synchronized, AtomicBoolean, etc) so there’s a chance that when it’s set to false, the change won’t be seen in the thread. - Here,
stopis an AtomicBoolean, fixing the issue in problem #1. That’s good. However, ifdoSomething()throws an exceptiondone.put("DONE")will never execute anddone.take()will wait forever. - Here,
Thread.run()is called rather thanThread.start(). The so-called “BackgroundTask” will actually run in the “foreground” ConcurrentHashMap.keySet().toArraywill give a “weakly consistent” snapshot of the keyset at the time of the call. By the time it returns, the keys in the map may have changed completely.- Oops. A string literal is used for the lock meaning that all instances of
Stackwill most likely share the same lock. - Oops. A new “lock” is created every time
push()is called, which is just as good as no lock. - Here
valuesis mercifully synchronized, but the varsizeisn’t. That is, ifsizeis used in any other methods it may not be synchronized correctly. - A
do/whileis used for the condition variable rather than justwhile. Ifcondis in a signaled state initially, the condition thatsizeis zero may not be checked. - If
out.close()throws an exception, themyLock.unlock()will not be executed. - Here, a string is being added to a blocking queue within a UI event handler. If the queue is full,
queue.put()will block causing the UI to become unresponsive. - I’m not totally sure with this one. I can think of a few things that could go wrong. First, if a listener is added or removed while
fireListeners()is executing, you could get aConcurrentModificationException. I think so anyway. I wasn’t able to find any indication of howArrayBufferiteration handles this. Second, since the listeners are notified with the lock held, you’ll almost certainly get a deadlock eventually, usually a lock inversion with some other thread that’s callingSwingUtilities.invokeAndWait(). - Ridiculously,
SimpleDateFormatis NOT THREADSAFE! So using the same formatter from multiple threads is a recipe for sadness.
I wonder what I missed.
Cheers!
11 should use CopyOnWriteArrayList in order to allow a listener to call add/remove listener when the event is being dispatched without causing a CME and/or deadlock.
7 is synchronizing on a non-final variable. Not all method calls will be synchronized on the same lock.
Esko. Good catch! Thanks.