0

What I am trying to do is to implement a key-specific read-write lock. Multiple read requests can be executed concurrently if there is no write request on that key. Put requests on different keys can be executed concurrently. I used a ConcurrentHashMap to save the key and keep a record of running write operations for each key.

My code looks like this: 

ConcurrentHashMap<String, AtomicInteger> count;
...
...
public void getLock(){
    synchronized (count.get(key)) {
        while (count.get(key).get() != 0) { // this means there are GET                                     
                                          requests running
            try {
                count.get(key).wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

The idea is that when a new thread wants to read, it needs to first check if there's any write on that key (if the count is not 0) and if not, it can go ahead, if yes, it needs to wait. So I suppose I have to use count.get(key).wait();. However, Java forces me to synchronized (count.get(key)) in order to use the wait() method.

I wonder does it make sense to use the synchronization here since I already use AtomicInteger?

p.s. I do have notify() later in the unlock method.

wwwwan
  • 407
  • 1
  • 4
  • 12
  • You're gonna have to explain what you want because this code is bizarre – Sean F Mar 28 '19 at 01:16
  • 2
    https://stackoverflow.com/questions/2779484/why-must-wait-always-be-in-synchronized-block?? – flycash Mar 28 '19 at 01:24
  • 2
    If you want a key-specific read-write lock, why not use a `Map`? – Slaw Mar 28 '19 at 01:31
  • I *think* you might want something like a `Semaphore`, or maybe a `CountDownLatch`? You definitely *don't* want to `wait()` on the monitor lock of an `Atomic*` class, that's... well, bizarre is a pretty good word for it. – Daniel Pryden Mar 28 '19 at 01:34
  • @Slaw thanks, i didn't even know there is a class like this – wwwwan Mar 28 '19 at 01:41
  • 1
    Your code is broken. If `count.get(key)` changes between the moment you synchronize and the moment you wait (because another thread changed the value of `key`) you will get an `IllegalMonitorStateException` because you're trying to `wait` on an Object that you don't hold the synchronized lock for. You need to assign `count.get(key)` to a local variable for this to work. – Erwin Bolwidt Mar 28 '19 at 01:47
  • And if you're gonna wait on *any* object, you must synchronize on it first. You don't have the choice. – user207421 Mar 28 '19 at 01:50

1 Answers1

0

I just realized why I still need a synchronized block for AtomicInteger. All comments as well as this link is pretty useful.

If the waiter didn't synchronize, then any old bit of code might change the predicate just before it goes to sleep, and then we're certainly in trouble.

So even it's AtomicInteger (actually, the datatype of the value doesn't really matter), just before it goes to wait, another thread can change its value and it would be wrong.

wwwwan
  • 407
  • 1
  • 4
  • 12