0

I'm working with a couple of classes which I need to use with multithreading. Since the access pattern is the same but they use different data structures, I extracted the access pattern to a superclass and left use its methods to manage the access, while the subclassess simply manage the data structure. It worked fine as long as I simply used synchronized blocks (and even single waits), but when I tried to introduce notify or notifyAll instructions it started throwing IllegalMonitorStateExceptions.

My superclass:

public abstract class SyncObject {
    protected Integer readLock;
    protected Boolean writeLock;

    public SyncObject(){
        this.readLock = 0;
        this.writeLock = false;
    }

    protected void acquireReadLock(){
        boolean hasBeenInterrupted = false;
        synchronized (this.writeLock){
            while(this.writeLock)
                try {
                    this.writeLock.wait();
                } catch (InterruptedException e) {
                //We can't leave the locks in a non-consistent state, so the interruption is swallowed
                //Once the lock have been set the interruption flag is set again to true, in case we need
                // it later
                    hasBeenInterrupted = true;
                }

            synchronized (this.readLock){
                this.readLock++;
            }
        }

        if(hasBeenInterrupted){
            Thread.currentThread().interrupt();
        }
    }

    protected void releaseReadLock(){
        synchronized (this.readLock){
            this.readLock--;
        }
    }

    protected void acquireWriteLock(){
        boolean hasBeenInterrupted = false;
        synchronized (this.readLock){
            synchronized (this.writeLock){
                while(this.readLock > 0)
                    try {
                        this.readLock.wait();
                    } catch (InterruptedException e) {
                        //See above
                        hasBeenInterrupted = true;
                    }

                while(this.writeLock)
                    try {
                        this.writeLock.wait();
                    } catch(InterruptedException e) {
                        //See above
                        hasBeenInterrupted = true;
                    }

                this.writeLock = true;
            }
        }

        if(hasBeenInterrupted){
            Thread.currentThread().interrupt();
        }
    }

    protected void releaseWriteLock(){
        synchronized (this.writeLock){
            this.writeLock = false;
        }
    }
}

My typical use of this is:

public class MySyncClass extends SyncObject{
   private String myData;

   public MySyncClass(){
       super();
       this.myData = "foo";
   }


   public String read(){
       super.acquireReadLock();
       String s = this.myData;
       super.releaseReadLock();
       return s;
   }

   public void write(String s){
       this.acquireWriteLock();
       this.myData = s;
       this.releaseWriteLock();
   }

}

If I copy the superclass' methods in my classess it works fine, but is there a way to manage everything from the superclass (thus avoiding to copy the same code in multiple classes)?

frollo
  • 1,296
  • 1
  • 13
  • 29
  • 1
    Locking on Booleans and integers like this while changing them is inherently broken, see https://stackoverflow.com/q/10324272/217324 – Nathan Hughes Aug 31 '17 at 08:42
  • Why not just use `java.util.concurrent.locks.ReentrantReadWriteLock` ? – Maxim Ponomarev Aug 31 '17 at 08:44
  • As I see, your code is destined to a deadlock situation anyway. – alirabiee Aug 31 '17 at 08:45
  • The linked answer only covers Boolean but the principle is the same. The thing you're locking on is an immutable value, when you change it you're swapping out locks and entirely messing up how locking works. – Nathan Hughes Aug 31 '17 at 09:20

0 Answers0