4

I have a synchronization exercise where i need to synchronize a read method so that any number of threads can execute it as long as no write methods are being executed. This has to be done from scratch, so i cannot use java.util.concurrent.locks ect.

For this i need some sort of mechanism to guard, but not block the read method, so the reading thread is blocked by a write, but not by other reads. I can't use a normal lock for this because invoking the lock method in the read method would cause other read threads to wait.

The rules should be as such: When a thread is inside write(), no other threads must enter read() or write() When a thread is inside read(), no other threads must enter write(), but they may enter read()

I have tried building a couple of homemade locks to deal with this problem. WriteLock is a fairly standard reenterant lock, except that it blocks if read is being executed (Using the readcounter) ReadLock should only cause threads to wait, if write() is being traversed. Otherwise it should simply allow the thread to go about its business and increment WriteLocks counter.

Code:

package sync;

public class SyncTest {
    Long testlong = new Long(0L);
    int reads = 0;
    int writes = 0;
    WriteLock w = new WriteLock();
    ReadLock r = new ReadLock(w);

    public SyncTest() {
        // TODO Auto-generated constructor stub
    }

    public static void main(String args[]){

        final SyncTest s = new SyncTest();

        for(int i = 0 ; i<3 ; i++){ //Start a number of threads to attack SyncTest
            final int ifinal = i;
            new Thread(){
                int inc = ifinal;
                @Override
                public void run() {
                    System.out.println("Starting "+inc);
                    long starttime = System.currentTimeMillis();
                    try {
                    while(System.currentTimeMillis()-starttime < 10){

                        if (inc < 2){

                            s.readLong();

                        }else{
                            s.writeLong(inc+1);
                        }
                    }
                    System.out.println(inc + " done");
                    if(inc == 0){
                        Thread.sleep(1000);
                        System.out.println(s.reads+" "+s.writes);
                    }
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }

                    // TODO Auto-generated method stub

                }
                @Override
                public String toString() {
                    // TODO Auto-generated method stub
                    return "Thread "+inc+" "+super.toString();
                }



            }.start();
        }
    }

    public Long readLong() throws InterruptedException{

        Long l;
        r.lock(); //Lock for reading
        //System.out.println("Read "+reads);
        l =  testlong;
        reads++;
        r.unlock(); //Unlock after reading
        return l;   
        }

    public void writeLong(int i) throws InterruptedException{

        w.lock(); //Lock for writing
        //System.out.println("Write "+writes);
        int curreads = reads;
        int curwrites = writes;
        testlong = testlong + i;
        writes++;

        Thread.sleep(100); //Simulate long write
        if(curreads != reads){
            System.out.println("Reads did not lock");
        }

        if(curwrites+1 != writes){
            System.out.println("Writes did not lock");
        }
        w.unlock(); //Unlock writing
    }

    protected class WriteLock{
        boolean isLocked = false;
        Thread lockedBy = null;
        int lockedCount = 0;
        int readers = 0; //The number of readers currently through the reading lock.

        public synchronized void lock() throws InterruptedException {
            System.out.println("Locking: "+Thread.currentThread());
            Thread callingThread = Thread.currentThread();
            while ((isLocked && lockedBy != callingThread) || readers > 0) { //Wait if locked or readers are in read()
                wait();
            }
            isLocked = true;
            lockedCount++;
            lockedBy = callingThread;
            System.out.println("Is locked: "+Thread.currentThread());
        }

        public synchronized void unlock() {
            //System.out.println("Unlocking: "+Thread.currentThread());
            if (Thread.currentThread() == this.lockedBy) {
                lockedCount--;

                if (lockedCount == 0) {
                    System.out.println("Is unlocked: "+Thread.currentThread());
                    isLocked = false;
                    notify();
                }
            }
        }

    }

    protected class ReadLock{
        WriteLock lock;

        public ReadLock(WriteLock lock) {
            super();
            this.lock = lock;
        }

        public synchronized void lock() throws InterruptedException { //If write() then wait
            System.out.println("Waiting to read: "+Thread.currentThread());
            Thread callingThread = Thread.currentThread();
            while (lock.isLocked && lock.lockedBy != callingThread) {
                wait();
            }
            lock.readers++; //Increment writelocks readers
            System.out.println("Reading: "+Thread.currentThread());

        }

        public synchronized void unlock() {
            lock.readers--; //Subtract from writelocks readers
            notify();
        }

    }

}

This is not working however, the reading lock works so far that it locks readers when a thread is writing, but it doesn't release them when WriteLock unlocks, as far as i can tell.

Is this just not conceptually sound, or is there something i don't understand with monitors? Or something else?

2
  • 2
    I'd encourage you to wrap try {} finally {} around your locks. If anything throws during the lock process, your code would not unlock. Commented Feb 1, 2013 at 14:39
  • Very true. But this is just a short-compilable and i didn't want to clutter the code. No errors are thrown. Commented Feb 1, 2013 at 14:46

2 Answers 2

15

(Answered before the question was edited in terms of it being an exercise.)

It sounds like you want a ReadWriteLock implementation.

A ReadWriteLock maintains a pair of associated locks, one for read-only operations and one for writing. The read lock may be held simultaneously by multiple reader threads, so long as there are no writers. The write lock is exclusive.

One implementation is ReentrantReadWriteLock.

For concurrency in particular, it's always worth looking in the existing libraries (java.util.concurrent et al) before you try to implement your own code. If you're anything like me, even if you can make it correct in terms of concurrency, it won't be as efficient as the code written by experts... and of course it's all work to start with ;)

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

3 Comments

Unless you want to implement a reader writer lock as a learning exercise, you should just use the one that comes with the java libraries that Jon suggested.
@MartinNielsen: You should have said so in your question. You said you had a synchronization problem. A ReadWriteLock is the solution to that synchronization problem. You said nothing about having to implement it from first principles.
Can not agree with this more. I know a lot of folks suffer from Not Invented Here Syndrome, but concurrency is hard enough; we don't need to make it harder by using non-battled-hardened libraries. (Yes, this guy happened to have to write it from scratch, but many people searching here perhaps won't be.)
1

Your ReadLock and WriteLock are synchronizing on different objects and calling wait and notify on different objects.

This allows the ReadLock to modify the counts in WriteLock while WriteLock is validating them. It will also cause the different locks not to wake from the calls to wait.

If you modify your ReadLock to use the WriteLock as a monitor you're get better results (I didn't check if this was the only problem)

protected class ReadLock{
    WriteLock lock;

    public ReadLock(WriteLock lock) {
        super();
        this.lock = lock;
    }

    public void lock() throws InterruptedException { //If write() then wait
        synchronized (lock) {
           System.out.println("Waiting to read: "+Thread.currentThread());
           Thread callingThread = Thread.currentThread();
           while (lock.isLocked && lock.lockedBy != callingThread) {
               lock.wait();
           }
           lock.readers++; //Increment writelocks readers
           System.out.println("Reading: "+Thread.currentThread());
       }
    }

    public void unlock() {
        synchronized (lock) {
           lock.readers--; //Subtract from writelocks readers
           lock.notify();
        }
    }

}

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.