This is the kind of bug that will drive a programmer crazy. It will appear only when the thread
scheduling is just so. You will write tests that will pass. All of the QA tests for the program
will pass. The program might run fine for a long time. But someday, just the right sequence of
events will occur and your program will give the wrong answer. And it will drive you crazy
trying to find out why.
Nor are things much better if we think about one thread doing the updating while another
thread returns a list of the Player objects. Imagine what happens if the Player object returned
is in the midst of being updated. If the counter for hits or number of bases has been incre-
mented but the number of atBats has not, then the statistics returned for the particular player
will be off. It won't be that the statistics will simply be out of date; they will be inconsistent.
This is not as bad as the earlier problems relating to dueling updates, as it will not corrupt the
data, only return inconsistent data that will later be consistent. But for a package that is sup-
posed to deal with such statistics, this is still not a happy result.
The way to deal with these problems is to use the synchronized keyword, which will place
a lock on either a method or an object to ensure that only one thread will have access to the
thing locked at any time. This gives us the tool that we need to make our code safe, but much
of the result has to do with the application of the tool, which requires thinking.
A naïve approach would be to simply synchronize all of the methods that can be called from
some client. This would mean, in the case of the StatRecorderImpl , that the getRoster()
and recordGame() methods be declared as synchronized . But this would be a less than op-
timal approach because it is too broad (it would mean that you couldn't get the roster of two
different teams at the same time).
What we really want to do is synchronize on the data structures that are going to get us into
trouble. This means that we will place a lock on those data structures, and for the code that
might cause trouble to run, it must get the lock. If all of the troublesome code is synchronized
on the same data, then only one part of the code can run at a time. This is how we avoid these
problems. But this is also tricky—if you lock for too long, you can kill your performance and
increase the possibility of deadlock. Deadlock is the condition where two threads each hold a
lock, and each needs a lock that the other is holding to proceed. Avoiding deadlock requires
that you make sure that you don't lock too much (among other things).
In our example, we could decide to synchronize on the Player object while doing the update.
But the updates happen at a lower level of granularity, so we should probably lock at that
lower level. One way to approach our update would be to rewrite our updateBatting() meth-
od along the lines of: