Is String get/set threadsafe?

Question

Let's say I have the following,

public class Foo{
    private String bar;

    public String getBar(){
        return bar;
    }

    public void setBar(String bar){
        this.bar = bar;
    }
}

Are these methods automatically threadsafe due to the immutable nature of the String class, or is some locking mechanism required?

Solution 1

No, this is not threadsafe. Foo is mutable, so if you want to ensure that different threads see the same value of bar – that is, consistency – either:

• Make bar volatile, or
• Make the methods synchronized, or
• Use an AtomicReference<String>.

The reads and writes of bar are themselves atomic, but atomicity is not thread safety.

http://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html

---

For in-depth coverage of Java concurrency, grab a copy of Java Concurrency in Practice (aka JCIP).

Solution 2

You're setting references, and as such String's immutability doesn't come into play. You're not affecting the contents of String.

Solution 3

No, not safe.

This is Foo mutable behavior; String's immutability does not accrue to Foo.

public class Foo{
    private String bar;

    public synchronized String getBar(){
        return bar;
    }

    public synchronized void setBar(String bar){
        this.bar = bar;
    }
}

Solution 4

No, it's not thread safe.

While String is immutable, the issue comes from the field of Foo. To make this more apparent, consider for example a method whose job would be to append (rather than replace) the value of bar. When it's called from multiple threads, some writes could be lost. The same (lost writes) can happen with your simple setter too, even if it's not obvious initially in this case.

View more solutions

Comments

[Eng.Fouad]

+1 Or simply declare bar as final, so the reference cannot be re-assigned to another value. Of course, there will be no setter then :)

[Eng.Fouad]

+1 What about synchronizing getBar()? Maybe some thread setting a new value to bar, while the other threads reading bar.

[duffymo]

That's immutable. No need for a setter in that case. And you have to be sure to supply a constructor to initialize the value, because you can't change it once you exit the constructor in that case.

[duffymo]

Sorry, you're making no sense to me. I think your comments are only confusing the issue.

[Matt Ball]

@duffymo does a non-synchronized getter with a synchronized setter guarantee consistency across threads?

[Clockwork-Muse]

... Out of curiosity, do either of those actually matter? I mean, if your actual problem is a race condition, that can still happen, despite the volatile or synchronized - it just decreases the 'window' in which it can happen, right? Or am I missing something here?

[Matt Ball]

@Clockwork-Muse and what race condition would that be? TBH, questions like these are why I avoid sharing mutable (non-concurrent-library) objects across threads as much as possible.

[Clockwork-Muse]

Thread one is updating bar continuously. Thread two is waiting for bar to read as some value (just spinning with a while(...) loop, say). The race condition is for thread two to read the value of bar before thread one updates it again. While either of those options make it so that all threads are guaranteed to have the same internal reference (although references are atomic regardless), that doesn't matter if they're working on the object at different time-slices. And yeah, stuff like this is why I prefer immutable objects.

[Steve Kuo]

@duffymo as you have it now, the getter needs to be synchronized to ensure that bar's current state is visible to all threads

[gstackoverflow]

@Matt Ball if instead String field was mutable, then volatile was not make class thread safe?