Question

I have read Java String class is immutable and thread-safe but I am still confused about whether the reference assignment for Strings is thread-safe or not.

First question: If thread A calls Foo.setString() while thread B calls Foo.getString(), is following code thread-safe?

Class Foo {
    String aString;
    public String getString() {
        return aString;
    }
    public void setString(s) {
        aString = s;
    }
}

Second question: If the above code is not thread-safe, using a ReentrantLock, how do I write the Foo.getString() method?

Class Foo {
    String aString;
    ReentrantLock aLock;
    public String getString() {
        aLock.lock();
        return aString;
        aLock.unlock(); // This line will be unreachable. How to fix??
    }
    public void setString(s) {
        aLock.lock();
        aString = s;
        aLock.unlock();
    }
}

I have to use a ReentrantLock because I need to use the tryLock(timeout) feature.

Was it helpful?

Solution

Question 1: It depends what you mean by thread safe, Eric Lippert wrote a very good blog post about it here... But declaring aString as volatile will mean that any reads on different threads are guaranteed to be correct.

Question 2:

Use a try... finally construct, unlocking in the finally block eg:

public String getString() {
  try {        
    aLock.lock();
    return aString;
  } finally {
    aLock.unlock();
  }
}

public void setString(s) {
  try {        
    aLock.lock();
    aString = s;
  } finally {
    aLock.unlock();
  }
}

OTHER TIPS

Making aString volatile is all you need in this case (i.e. a read after a write will always see the newest value) - making the method synchronized isn't especially useful since we're only executing a single statement - in both cases so you'd need a higher level synchronization anyhow if you want to do more work (say "If String is XYZ then assign XYA to it"). I.e. both synchronized and volatile give you the same guarantees (apart from the obvious difference if we use synchronized elsewhere in the class)

So volatile is enough and more performant, but synchronized is just as fine - the only performance difference you could notice was if you had lots of contended accesses, but otherwise it doesn't matter - synchronized is extremely optimized for non contended accesses on at least Hotspot (but I assume the same is true for all modern JVMs).

Use synchronized keyword :

   public synchronized String getString() {
        return aString;
    }

    public synchronized void setString(s) {
        aString = s;
    }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top