3

I have a scenario where I want to use StringBuilder as a local variable in a method. I understand if StringBuilder is a local method variable it should not have any problems about thread-safety.

However, if I append to the StringBuilder an instance variable like:

class MyClass {
    private List<String> property;

    public void myMethod() {
        StringBuilder sb = new StringBuilder();
        for(String s : property) {
            sb.append(s);
        }
    }

    // some other methods that mutate property
}

I think to make this thread-safe, simply change the StringBuilder to StringBuffer is not sufficient. Shall I synchronize on the property itself?

4
  • what is my method doing? seems it's creating a StringBuilder object and throwing it away... Commented Jun 4, 2013 at 13:20
  • 1
    You need to synchronize the access to the List, not the StringBuilder Commented Jun 4, 2013 at 13:20
  • You probably want to separator between your properties. Commented Jun 4, 2013 at 18:13
  • might be interesting: How do I prove programmatically that StringBuilder is not threadsafe? Commented Feb 19, 2018 at 10:30

4 Answers 4

8

It's not StringBuilder but List<String> property who is in danger. You have two options:

  1. Make myMethod and other methods that mutate property synchronized

  2. Use java.util.concurrent.CopyOnWriteArrayList which is thread safe and makes a snapshot for iterator

Sign up to request clarification or add additional context in comments.

1 Comment

Just an FYI for the OP: if you plan on making a lot of updates, be wary of the CopyOnWriteArrayList approach; in that case you would be better off going with option 1.
4

The problem is that while you are calling yours myMethod(), another thread can be adding new String to yours property list, thus, modifying the result.

One way to avoid this is using synchronized in the methods that access property in any way, or you can use the good old ReadWriteLock. The implementation most used is the ReentrantReadWriteLock. There are good examples in the internet, but what you would do is something like:

class MyClass {
    private final ReadWriteLock propertiesLock = new ReentrantReadWriteLock();
    private final Lock read = propertiesLock.readLock();
    private final Lock write = propertiesLock.writeLock();
    ... 

    public StringBuilder myMethod() {
        StringBuilder builder = new StringBuilder();
        read.lock();
        try {
           // your writing here.
        } finally {
            read.unlock();
        }
        return builder;
    }

    public void addProperty(String property) {
        write.lock();
        try {
            properties.add(property);
        } finally {
            write.unlock();
        }
    }
}

Comments

3

You can synchronize on property, or on any object, as long as you synchronize on the same object in wherever you access property.

An alternative to synchronization would be using a thread-safe implementation of List, such as CopyOnWriteArrayList, which is great (fast, less contention than with synchronization) if the list is read often but not modified too often.

Comments

2

It seems that the synchronize issue lies on property.

So just add a synchronization on it:

synchronized (property) {
   ...
}

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.