Writing buffers to a Java channel: Thread-safe or not?

Question

Consider the following code snippet, which simply writes the contents of someByteBuffer to the standard output:

// returns an instance of "java.nio.channels.Channels$WritableByteChannelImpl"
WritableByteChannel w = Channels.newChannel(System.out);
w.write(someByteBuffer);

Java specifies that channels are, in general, intended to be safe for multithreaded access, while buffers are not safe for use by multiple concurrent threads.

So, I was wondering whether the above snippet requires synchronization, as it is invoking the write method of a channel (which is supposed to be thread-safe) on some buffer (which is not thread-safe).

I took a look at the implementation of the write method:

public int write(ByteBuffer src) throws IOException {
    int len = src.remaining();
    int totalWritten = 0;
    synchronized (writeLock) {
        while (totalWritten < len) {
            int bytesToWrite = Math.min((len - totalWritten),
                                        TRANSFER_SIZE);
            if (buf.length < bytesToWrite)
                buf = new byte[bytesToWrite];
            src.get(buf, 0, bytesToWrite);
            try {
                begin();
                out.write(buf, 0, bytesToWrite);
            } finally {
                end(bytesToWrite > 0);
            }
            totalWritten += bytesToWrite;
        }
        return totalWritten;
    }
}

Notice that everything is synchronized by the writeLock, except the first two lines in the method. Now, as the ByteBuffer src is not thread-safe, calling src.remaining() without proper synchronization is risky, as another thread might change it.

Should I synchronize the line w.write(someByteBuffer) in the above snippet, or am I missing something and the Java implementation of the write() method has already taken care of that?

Edit: Here's a sample code which often throws a BufferUnderflowException, since I commented out the synchronized block at the very end. Removing those comments will make the code exception free.

import java.nio.*;
import java.nio.channels.*;

public class Test {
    public static void main(String[] args) throws Exception {

        ByteBuffer b = ByteBuffer.allocate(10);
        b.put(new byte[]{'A', 'B', 'C', 'D', 'E', 'F', 'G', '\n'});

        // returns an instance of "java.nio.channels.Channels$WritableByteChannelImpl"
        WritableByteChannel w = Channels.newChannel(System.out);

        int c = 10;
        Thread[] r = new Thread[c];
        for (int i = 0; i < c; i++) {
            r[i] = new Thread(new MyRunnable(b, w));
            r[i].start();
        }
    }    
}

class MyRunnable implements Runnable {
    private final ByteBuffer b;
    private final WritableByteChannel w;

    MyRunnable(ByteBuffer b, WritableByteChannel w) {
        this.b = b;
        this.w = w;
    }

    @Override
    public void run() {
        try {
            // synchronized (b) {
                b.flip();
                w.write(b);
            // }
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Answer 1

The point is: if your setup allows more than one thread to tamper with that buffer object, then you are subject to threading issues. It is that simple!

The question is not if channel.write() is thread-safe. That is good to know, but not at the core of the problem!

The real question is: what is your code doing with that buffer?

What does it help that this channel implementation does lock internally on something when the data it is operating on ... is coming from the outside?!

You know, all kinds of things could happen on that src object coming into this method - while that channel is busy writing the buffer!

In other words: the question whether this code is "safe" fully depends on what your code is doing with that very src buffer object in parallel.

Given the OP's comment: the core point is: you have to ensure that any activity that makes use of that byte buffer is thread-safe. In the example given, we have two operations:

b.flip();
w.write(b);

Those are the only operation each thread will be doing; thus: when making sure that only one thread can make those two calls (as shown; by looking on the shared buffer object); then you are good.

It is really simple: if you have "shared data"; then you have to ensure that the threads reading/writing that "shared data" are synchronized somehow to avoid race conditions.

Answer 2

You only need to lock the Channel if you are making multiple writes from multiple threads and want to ensure atomicity of those writes in which case you need to lock the System.out object.

If you shared a mutated data structure which is not thread safe across threads, you need to add locking. I would avoid using a ByteBuffer in multiple threads if you can.