Thursday, May 08, 2008

Bad concurrency advice: interned Strings

I just read Thread Signaling from Jacob Jenkov. It is fine as far as it goes to introduce the reader to Object.wait() and Object.notify().

But it has one fatal flaw: it uses a literal java.lang.String for coordinating between threads. Why is this wrong?

Strings are interned by the compiler. To quote the javadocs: All literal strings and string-valued constant expressions are interned. Using a literal string means that any other code anywhere in the JVM, even in other libraries, which use the same String literal value all share the same object for wait() and notify(). This code:

public void dastardly() {
    "".notify();  
}

will wake up a thread waiting on empty string, including one in utterly unrelated code.

Don't do that. Instead, create a fresh Object for coordinating threads. This age-worn advice for lock objects (synchronize(lock)) applies just as much to objects used to coordinate threads.

3 comments:

Jakob said...

yes, indeed a "fatal" flaw... not :-) The only real problem is that you risk waking a few too many threads, that weren't supposed to
wake up. And what exactly does that equal? Yes, that's right, a Spurious Wakeup! ... That is why we wrap the wait() call in a while loop.
So, though I agree it is bad practice to use String's as monitor objects, it is by no chance "fatal". Nothing will happen except
you risk waking up a few threads that will then go back to waiting immediately.

I fixed the text now though, it will be online on monday or so.

Anonymous said...

+1 with binkley for the fatal flaw :) Process coordination on, effectively, a global shared object is a Bad Idea.

Processes should only sync on the objects they're interested in. Spurious Wakeups could cause extremely subtle bugs related to timing.

Jakob said...

schoenobates, why don't you enlighten us all by explaining rather than just stating, what these timing errors are? I can't think of anything that a spurious wakeup could destroy, so personally I'd be happy to learn something new. If a spurious wakeup could destroy your application, you are f....d, cause they are bound to happen sooner or later.