Necessity of synchronized on local variable

Question

In the JSON-java library (org.json.JSONArray) I have found this code snippet with a synchronized block around a method-local variable

public String toString(int indentFactor) throws JSONException {
    StringWriter sw = new StringWriter();
    synchronized (sw.getBuffer()) {
        return this.write(sw, indentFactor, 0).toString();
    }
}

I do not understand the necessity for the synchronization here, as the StringWriter is only local to the given method (and, why the synchronization is on the Buffer). Is the synchronization really necessary here, and if, why?

It is unnecessary. With the default constructor, the buffer is a newly instantiated object. `buf = new StringBuffer();` inside the constructor. — Sotirios Delimanolis, Aug 19 '13 at 16:03
Note that the author of the org.json library is a JavaScript programmer, not a Java programmer, and this fact is demonstrated throughout the code. For serious JSON processing, consider using Google's Gson library. — C. K. Young, Aug 19 '13 at 16:07

score 8 · Accepted Answer · answered Aug 19 '13 at 16:07

8

This may be a performance optimization. In the oracle jvm, re-acquiring an already held lock is very fast. Presumably, the write call is making numerous calls into the StringBuffer. By locking before calling write, the lock will be held through all of those calls instead of being released and re-acquired for each call.

answered Aug 19 '13 at 16:07

jtahlborn

52,909
5
76
118

As of Java 6, lock biasing means that acquiring an uncontended lock is fast anyway. – C. K. Young Aug 19 '13 at 16:08
@ChrisJester-Young - yes, i don't know how relevant that particular performance optimization still is (or ever was). – jtahlborn Aug 19 '13 at 16:09
I don't understated _being released and re-acquired for each call_. The `write()` call doesn't try and `synchronize` on the `Writer` at all. The lock gets acquired and released only once, inside the `toString(int)` method in OP's question. – Sotirios Delimanolis Aug 19 '13 at 17:56
1

@SotiriosDelimanolis - all the methods of StringBuffer (which underlies the StringWriter) are synchronised. – jtahlborn Aug 19 '13 at 19:27

score 6 · Answer 2 · answered Aug 19 '13 at 16:05

The empty constructor for StringWriter is

/**
 * Create a new string writer using the default initial string-buffer
 * size.
 */
public StringWriter() {
    buf = new StringBuffer();
    lock = buf;
}

Nothing is shared, therefore the synchronized block is unnecessary.

Unless... write delegates to another thread, but I seriously doubt that.

DaoWen · Answer 3 · 2013-08-19T16:16:05.050

getBuffer() returns a StringBuffer, and according to the documentation a StringBuffer is already synchronized:

String buffers are safe for use by multiple threads. The methods are synchronized where necessary so that all the operations on any particular instance behave as if they occur in some serial order that is consistent with the order of the method calls made by each of the individual threads involved.

This means that synchronizing again on the StringBuffer is totally overkill. Changes to the StringWriter will automatically be synchronized because it uses the synchronized StringBuffer internally.

Since the StringWriter instance is local to the method call, it's impossible to have multiple threads accessing the same instance at the same time, which also makes synchronization unnecessary.

score 0 · Answer 4 · answered Dec 03 '14 at 13:50

It is a bug. Every thread creates its own local variable in the method and synchronizes on it.Each time entering the method threads create their own object-monitor which cannot be held by another thread because it's local and lives only on thread's stack!

Necessity of synchronized on local variable

4 Answers4

Linked