views:

126

answers:

3

What is the point of the synchronization here? Why not just use: mConnectedThread.write(out); ?

The code snippet is from the BluetoothChat sample for Android (found here) http://developer.android.com/resources/samples/BluetoothChat/index.html

    /**
 * Write to the ConnectedThread in an unsynchronized manner
 * @param out The bytes to write
 * @see ConnectedThread#write(byte[])
 */
public void write(byte[] out) {
    // Create temporary object
    ConnectedThread r;
    // Synchronize a copy of the ConnectedThread
    synchronized (this) {
        if (mState != STATE_CONNECTED) return;
        r = mConnectedThread;
    }
    // Perform the write unsynchronized
    r.write(out);
}
+1  A: 

Synchronization is needed to ensure that you don't have inconsistent state.

Without the synchronization, the code would be:

public void write(byte[] out) {
    if (mState != STATE_CONNECTED) return;
    mConnectedThread.write(out);
}

Now, if the connection where to close between the if statement check and the method invocation, mConnectedThread might be assigned to null, before the method invocation execution. That would result into a NullPointerException.

notnoop
Of course, without knowing more the connection could still be closed between the end of the sync block and the actual write() call... which might now just produce an entirely different error. And, if ConnectThread handles write()s after cancel gracefully then simply assigning it to a local variable and checking for null would be an ever easier way.
PSpeed
+1  A: 

Whenever two threads access the same data (here, the variables mState and mConnectedThread), they must use a "memory barrier" that ensures visibility. It appears that the atomicity of mState and mConnectedThread are important here too.

Visibility means that changes made by one thread will be visible to another. Optimizations might cause a value to be cached so that changes are only visible to the thread that made them. Synchronization causes any writes to be reflected in main memory, and any reads to bypass local caches and be made against main memory. In theory, without synchronization one thread could set mState and mConnectedThread, but other threads might never "see" those changes, and wait forever for that condition to change.

Atomicity means that multiple actions cannot be observed individually. From another thread's point of view, none of the changes have occurred, or all of the changes have occurred. So, for example, another thread could never see that mState is STATE_CONNECTED, but read mConnectedThread before it has been assigned.

erickson
A: 

The lock is needed for consistency. Copying mConnectedThread to a separate variable is because then the write - which is a potentially length operation - can be done outside of the lock.

Rovpedal
Spot the C programmer ;) (didn't vote you down)
Kristopher Ives