> I wrote some code to get a message out of a hash table. The message was going to be put there by another thread. There was a small chance of a race where it wouldn’t be there yet when I initially looked for it. The code looked something like this:
> The wait() call blocks the thread, waiting for the notifyAll() from the thread that puts the message into the map. The 1000 means one second. The timeout was going to be on the order of five seconds.
> The above code is simple and correct
… it looks neither simple nor correct.
If this is multithreaded code, "small chance of a race" is a smell, and "notifyAll" is probably a smell. The multithreading primitive that I'd want to see here is a condition, so named because it permits a thread to wait for a particular condition. A condition wakes the "notified" thread with a mutex already locked, so that the condition ("is there a message in the table?") can be checked without interference from other threads. If there is a message, you can remove it, process it, etc. without races, and if there isn't, you can atomically release the lock and go back to sleep (which is again a thing the condition provides; it is not two separate calls).
> The above code is simple and correct
… it looks neither simple nor correct.
If this is multithreaded code, "small chance of a race" is a smell, and "notifyAll" is probably a smell. The multithreading primitive that I'd want to see here is a condition, so named because it permits a thread to wait for a particular condition. A condition wakes the "notified" thread with a mutex already locked, so that the condition ("is there a message in the table?") can be checked without interference from other threads. If there is a message, you can remove it, process it, etc. without races, and if there isn't, you can atomically release the lock and go back to sleep (which is again a thing the condition provides; it is not two separate calls).