4
\$\begingroup\$

Description of the Readers–writers problem.

It looks like it works, but I have some doubts:

class ReadWriteBuffer<T> {

    private static final Object writeLock = new Object();

    private static final Object readWriteLock = new Object();

    private int numberReaders = 0;

    private int numberWriters = 0;

    private int numberWriteRequests = 0;

    public void write(T object) throws InterruptedException {

        numberWriteRequests++;

        synchronized (readWriteLock) {
            while (numberReaders > 0) {
                readWriteLock.wait();
            }
        }

        synchronized (writeLock) {

            numberWriteRequests--;

            numberWriters++;

            System.out.println("Writer #" + Thread.currentThread().getId() + " started writing.");
            Thread.sleep(3000);
            System.out.println("Writer #" + Thread.currentThread().getId() + " finished writing.");

            numberWriters--;

            synchronized (readWriteLock) {
                readWriteLock.notifyAll();
            }
        }
    }

    public void read() throws InterruptedException {

        synchronized (readWriteLock) {
            while (numberWriters > 0 || numberWriteRequests > 0) {
                readWriteLock.wait();
            }
        }

        numberReaders++;

        System.out.println("Reader #" + Thread.currentThread().getId() + " started reading.");
        Thread.sleep(1000);
        System.out.println("Reader #" + Thread.currentThread().getId() + " finished reading.");

        numberReaders--;

        synchronized (readWriteLock) {
            readWriteLock.notifyAll();
        }
    }
}

class Reader implements Runnable {

    private final ReadWriteBuffer<Object> buffer;

    private int priority;

    public Reader(int priority, ReadWriteBuffer<Object> buffer) {
        this.priority = priority;
        this.buffer = buffer;
    }

    @Override
    public void run() {
        while (true) {
            try {
                Thread.sleep(priority);
                buffer.read();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

class Writter implements Runnable {

    private final ReadWriteBuffer<Object> buffer;

    private int priority;

    public Writter(int priority, ReadWriteBuffer<Object> buffer) {
        this.priority = priority;
        this.buffer = buffer;
    }

    @Override
    public void run() {
        while (true) {
            try {
                Thread.sleep(priority);
                buffer.write("Object");
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

public class ReadersWriters {

    public static void main(String[] args) {

        ReadWriteBuffer<Object> sharedBuffer = new ReadWriteBuffer<Object>();

        new Thread(new Reader(1, sharedBuffer)).start();
        new Thread(new Reader(700, sharedBuffer)).start();
        new Thread(new Reader(100, sharedBuffer)).start();
        new Thread(new Writter(2000, sharedBuffer)).start();
        new Thread(new Writter(4000, sharedBuffer)).start();
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ What are your doubts? \$\endgroup\$
    – Jamal
    Aug 13, 2014 at 18:05
  • \$\begingroup\$ f.e. not sure about counters, seems that they should be atomic \$\endgroup\$
    – pvllnspk
    Aug 13, 2014 at 18:07

1 Answer 1

11
\$\begingroup\$

This code in the read() method is broken:

public void read() throws InterruptedException {

    synchronized (readWriteLock) {
        while (numberWriters > 0 || numberWriteRequests > 0) {
            readWriteLock.wait();
        }
    }

    numberReaders++;

    System.out.println("Reader #" + Thread.currentThread().getId() + " started reading.");
    Thread.sleep(1000);
    System.out.println("Reader #" + Thread.currentThread().getId() + " finished reading.");

    numberReaders--;

    synchronized (readWriteLock) {
        readWriteLock.notifyAll();
    }
}

Since numberReaders ++ and -- is not in a synchronized block, it essentially may as well not happen as far as other threads are concerned, or, worse, it could partially happen (like the ++ may happen, and the -- may not).

Additionally, they may not produce the right results, anyway. The ++ operator is not atomic, and it is possible for the value to be ++ in two different threads at the same time, and the result to be just one increment for both invocations.

Bottom line is that, from a multi-threaded perspective, this code is broken.

References:

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.