Why isn't the awaiting thread activated with a signalAll?

Question

I got 2 functions. The first one discoverHosts() sends an request message to other computers. After this it goes to sleep with the await command. A separate threat calls the handleMessage() function when he receives a response. After he handles the response he uses notifyAll() to let discoderHosts() know that he has to check that all responses are received.

DiscoverHosts() does await when he calls the function. However when the separate threat calls handleMessage(), discoverHosts() doesn't awake when handleMessage calls the signalAll(). I checked while debuggig if signalAll() is called and this is the case. I have almost the same bit of code somewhere else in my project where it does work.

Do any of you guys know what I am overlooking?

private final Lock lock = new ReentrantLock();
private final Condition allReceived = lock.newCondition();

private void discoverHosts() throws Exception {
    lock.lock();
    externalNodes = new HashMap<String, NodeAddress>();
    Message msg = new Message(null, "REQUEST_IP");
    logger.debug("Broadcasting ip request, waiting responses");
    channel.send(msg);
    // TODO:Write a time-out
    while (channel.getView().size() - 1 != externalNodes.keySet().size()) {
        logger.debug("Channel: " + (channel.getView().size() - 1));
        logger.debug("Responses: "+ externalNodes.keySet().size());
        allReceived.await();
    }
    logger.debug("All answers received");
    lock.unlock();
}

protected void handleMessage(Message msg) {
    lock.lock();
    if (!((String) msg.getObject()).matches("IP_RESPONSE:[0-9.]*"))
        return;
    logger.debug("Received answer from " + msg.getObject());
    String ip = ((String) msg.getObject()).replaceAll("IP_RESPONSE:", "");
    // externalHostIps.add(ip);
    NodeAddress currentAddress = new NodeAddress(ip, msg.getSrc());
    externalNodes.put(ip, currentAddress);
    logger.debug("Signalling all threads");
    allReceived.signalAll();
    lock.unlock();
    logger.debug("Unlocked");
}

Logger output:

4372 [main] DEBUG com.conbit.webhackarena.monitor.monitor.Monitor@3b91eb  - Broadcasting ip request, waiting responses
4372 [main] DEBUG com.conbit.webhackarena.monitor.monitor.Monitor@3b91eb  - Channel: 1
4372 [main] DEBUG com.conbit.webhackarena.monitor.monitor.Monitor@3b91eb  - Responses: 0
4394 [Incoming-1,webhackarena,leendert-K53SV-53745] DEBUG com.conbit.webhackarena.monitor.monitor.Monitor@3b91eb  - Received answer from IP_RESPONSE:192.168.1.106
4396 [Incoming-1,webhackarena,leendert-K53SV-53745] DEBUG com.conbit.webhackarena.monitor.monitor.Monitor@3b91eb  - Signalling all threads
4397 [Incoming-1,webhackarena,leendert-K53SV-53745] DEBUG com.conbit.webhackarena.monitor.monitor.Monitor@3b91eb  - Unlocked

`"string " + new Integer(n).toString()` is the same as `"string" + n` — Peter Lawrey, Apr 11 '13 at 16:51
@PeterLawrey not sure that entirely solves the locking problem. Ha. — Boris the Spider, Apr 11 '13 at 16:53
what is your logging output? IN any case, you should wrap `lock()` in try/finally Blocks. — Ralf H, Apr 11 '13 at 16:53
OK, back to the OP: You talk about calling `notifyAll()` and yet the code has `signalAll()`. Are you sure those two codes are equivalent? — Ralf H, Apr 11 '13 at 16:56
@RalfH `signallAll()` is the `Condition` version of `notifyAll()`. That part's OK. — Nathaniel Waisbrot, Apr 11 '13 at 16:57
I know that, but it sounded like the OP was remembering/comparing to another piece of code that worked. And this other piece seems different from this, so his comparison may not be valid. — Ralf H, Apr 11 '13 at 16:58
Always make sure that you do your `lock.unlock()` in a `finally` block so exceptions don't cause you to leek the lock. — Gray, Apr 11 '13 at 16:59

score 3 · Accepted Answer · answered Apr 11 '13 at 16:58

3

I think your problem is this line

if (!((String) msg.getObject()).matches("IP_RESPONSE:[0-9.]*"))
    return;

Which means under some condition you acquire the lock and never release it.

Always use try...finally blocks with Lock to avoid this issue.

protected void handleMessage(Message msg) {
    lock.lock();
    try {
        if (!((String) msg.getObject()).matches("IP_RESPONSE:[0-9.]*"))
            return;
        logger.debug("Received answer from " + msg.getObject());
        String ip = ((String) msg.getObject()).replaceAll("IP_RESPONSE:", "");
        // externalHostIps.add(ip);
        NodeAddress currentAddress = new NodeAddress(ip, msg.getSrc());
        externalNodes.put(ip, currentAddress);
        logger.debug("Signalling all threads");
        allReceived.signalAll();
    } finally {
        lock.unlock();
    }
}

answered Apr 11 '13 at 16:58

Boris the Spider

59,842
6
106
166

I made the assumption that he was getting to the `signalAll()` method call. If he is assuming that he gets there then you are prolly right. +1 – Gray Apr 11 '13 at 17:02
This did the trick, strange because I debugged and confirmed that signalAll() was called and also my logs confirm it. Thanx for the quick help. – Dommicentl Apr 11 '13 at 17:08
@Vogabe you don't have a log entry before the `return` - you most likely have more than one message handler. – Boris the Spider Apr 11 '13 at 17:13

score 1 · Answer 2 · answered Apr 11 '13 at 16:56

However when the separate threat calls handleMessage(), discoverHosts() doesn't awake when handleMessage calls the signalAll()

I suspect that you have two different instances of the class in question and since the Condition allReceived is private final, they are not actually dealing with the same condition.

If you are trying to debug this (or use System.out.println debugging), be sure that your instance of the wrapping class is the same in both threads.

Why isn't the awaiting thread activated with a signalAll?

2 Answers2