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.
stopis an AtomicBoolean, fixing the issue in problem #1. That’s good. However, if
doSomething()throws an exception
done.put("DONE")will never execute and
done.take()will wait forever.
Thread.run()is called rather than
Thread.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.
valuesis mercifully synchronized, but the var
sizeisn’t. That is, if
sizeis used in any other methods it may not be synchronized correctly.
do/whileis used for the condition variable rather than just
condis in a signaled state initially, the condition that
sizeis zero may not be checked.
out.close()throws an exception, the
myLock.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 a
ConcurrentModificationException. I think so anyway. I wasn’t able to find any indication of how
ArrayBufferiteration 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 calling
SimpleDateFormatis NOT THREADSAFE! So using the same formatter from multiple threads is a recipe for sadness.
I wonder what I missed.