G52CON 2008-2009 Coursework feedback
Prize
There were several very good solutions to the coursework, some of them along very
similar lines (some kind of locking at the hotel level, everything correct,
extended methods implemented), so it was difficult to distinguish between them with
respect to the prize. Of more unusual solutions, the best were by Azam Fahad
Alfayez, Daniel Kiss, and Thomas Bridge. I decided to award the prize to
Thomas Bridge because his solution
was very neat and simple and maximized concurrency.
Here are the solutions I talked about in lecture 20 :
Most students implemented a version of a fully synchronized solution or
a read-write lock solution, with the lock at the hotel level. There were
several good attempts at a fine-grained solution but all the ones I've seen
so far either realized that
locking only at the room level still left space for bad interleaving of
threads and
introduced a hotel-level locking for part of the method as well, or allowed
too much (unsafe)
concurrent access.
Most of the mistakes I've seen were general Java programming mistakes, and
going thorough them is not useful for the exam revision. Below is a list
of bugs related to concurrency that I have come across in the coursework
(more may be added in the week of the 18th of May, as well as the prize
winner announcement).
- A pretty bad bug: some of the methods use read and write locks
at the hotel level (for example,
roomBooked()
uses a read lock and
bookRoom()
uses a write lock) and other methods are synchronized
(for example
updateBooking()
and
cancelBooking()
). This means that there is
no mutual exclusion between the first two and the last two, since
the synchronized keyword only guarantees that the method is executed
with mutual exclusion with respect to other synchronized methods
(and
roomBooked()
and
bookRoom()
are not synchronized).
- The following was not counted as a bad bug, but shows a lack of
understanding which is best avoided in an exam:
methods are synchronized or use read-write locks at the
hotel level, but inside the methods there is a lot more mutual
exclusion-ensuring stuff going on, with
setting flag variables, using spinlocks (which don't work in Java anyway,
see lecture 10), using thread-safe datastructures such as Vectors
(rather than ArrayLists) and synchronized maps and so on.
If only one thread at a time is going to
modify the hotel data structures this is all absolutely unnecessary.
- calling wait() from non-synchronized code - this actually does not
compile (wait on which monitor?)
- Minor problem, not even a bug but a bad programming style: when
using a lock, not putting the subsequent code in a try{} block and the lock
release in a finally{} block. If you don't do that, if the code of the
method throws some uncaught exception after acquiring the lock, for example
as often happened an ArrayIndexOutOfBoundsException, the lock is never
released and no other thread can acquire it.
- When booking multiple rooms, acquiring the locks in arbitrary order
(for example, the order in which the rooms are passed to the method). If two threads
do this, and one wants to acquire locks on rooms 1,2 and another on rooms 2,1 and
each gets to lock the first room on their list, they will deadlock.
This file is maintained by Natasha Alechina, last modified
19:11, 26-May-2009.