CODEWITHSUNDEEP
javaconcurrency
JBT
JBT

Reputation: 8746

Condition instance signalAll() doesn't return

I have the following code:

@Log4j
public class ItemStore {
    final Lock lock = new ReentrantLock();
    final Condition hasItem = lock.newCondition();

    private Map<String, String> store = new Hashtable<>();

    public void put( String handle, String item) {
        store.put( handle, item );
        log.info("stored " + handle );
        hasItem.signalAll();
        log.info("signaled all threads");
    }

    public String fetchWithTimeout( String handle, long timeoutInSec ) throws InterruptedException {
        try {
            lock.lock();
            while ( !store.containsKey( handle ) ) {
                log.info("store doesn't have " + handle + "; keep waiting");
                hasItem.await( timeoutInSec, TimeUnit.SECONDS);
            }
            return store.get( handle );
        } finally {
            lock.unlock();
        }
    }
}



@Test
public void test_withPut() throws InterruptedException {
    ItemStore itemStore = new ItemStore();
    final String key = "foo";
    final String value = "bar";

    new Thread() {
        @Override
        public void run() {
            try {
                Thread.sleep(3000);
                log.info("slept 3 seconds");
                itemStore.put(key, value);
            } catch (Exception e) {
            }
        }
    }.start();

    log.info("fetching");
    String actual = itemStore.fetchWithTimeout(key, 20);
    log.info("actual = " + actual );
    assertEquals( actual, value );
}

Based on the logs from the test as below:

2014-10-05 17:52:48 INFO  com.tns.ct.downloader.tests.commons.ItemStoreTest.test_withPut():36 - fetching
2014-10-05 17:52:48 INFO  com.tns.ct.downloader.tests.commons.ItemStore.fetchWithTimeout():30 - store doesn't have foo; keep waiting
2014-10-05 17:52:51 INFO  com.tns.ct.downloader.tests.commons.ItemStoreTest.run():29 - slept 3 seconds
2014-10-05 17:52:51 INFO  com.tns.ct.downloader.tests.commons.ItemStore.put():21 - stored foo
2014-10-05 17:53:08 INFO  com.tns.ct.downloader.tests.commons.ItemStoreTest.test_withPut():38 - actual = bar

it seems that hasItem.signalAll() has never returned, as the signaled all threads log has never been issued. Another clue is that the program exited only when the 20 seconds timeout was reached. So, why is the signalAll() method blocked in this case?

Upvotes: 0

Views: 173

Answers (1)

JB Nizet
JB Nizet

Reputation: 692013

Quote from the documentation of signalAll():

An implementation may (and typically does) require that the current thread hold the lock associated with this Condition when this method is called.

Quote from the documentation of ReentrantLock.newCondition():

The returned Condition instance supports the same usages as do the Object monitor methods (wait, notify, and notifyAll) when used with the built-in monitor lock.

If this lock is not held when any of the Condition waiting or signalling methods are called, then an IllegalMonitorStateException is thrown.

Not sure why an IllegalMonitorException isn't thrown in your test, but what's sure is the the putting thread doesn't hold the lock when it calls signalAll() on the condition.

EDIT: as @Fildor mentions, an exception is probably thrown, but swallowed by the empty catch block in your test. Don't use empty catch blocks. If you threw a runtime exception wrapping the caught exception instead of swallowing it, the problem would become obvious.

Upvotes: 2

Related Questions