Is this the best message delay algorithm?

In my application I am attempting to queue outgoing messages by the following rules

• By default they should be sent no less than messageDelay apart

• Some messages can be sent immediately, completely bypassing all queued messages

• Additionally, some immediately sent messages can reset the waiting time of queued messages

If this is confusing, this is what it would look like on the wire

|  Normal  |Immediate |Immediate-reset|
M----------M---M------M----M----------M ....

This was originally handled by a dedicated OutputThread and queue but for various reasons outside of the scope of this question I am attempting to implement this with ReentrantLock and Condition so when any of the methods finally returns, the message is known to of been sent (or an Exception is thrown meaning it didn't)

However this is my first time using ReentrantLock and while I think I've implemented it correctly I'm needing someone to verify that its correct since this is an extremely difficult thing to test

protected final ReentrantLock writeLock = new ReentrantLock(true);
protected final Condition writeNowCondition = writeLock.newCondition();

public void sendRawLine(String line) {
    //[Verify state code]
    writeLock.lock();
    try {
        sendRawLineToServer(line);
        //Block for messageDelay. If rawLineNow is called with resetDelay
        //the condition is tripped and we wait again
        while (writeNowCondition.await(getMessageDelay(), TimeUnit.MILLISECONDS)) {
        }
    } catch (Exception e) {
        throw new RuntimeException("Couldn't pause thread for message delay", e);
    } finally {
        writeLock.unlock();
    }
}

public void rawLineNow(String line, boolean resetDelay) {
    //[Verify state code]
    writeLock.lock();
    try {
        sendRawLineToServer(line);
        if (resetDelay)
            //Reset the 
            writeNowCondition.signalAll();
    } finally {
        writeLock.unlock();
    }
}

Is this the correct way to im

I can see some problems with the code:

-
As @fge mentions, catching Exception is problematic. You should catch the exceptions that you are expecting to occur, and let any other unchecked exceptions propagate.

-
Wrapping the exceptions in RuntimeException is a bad idea unless it is really necessary. It is better to either let them propagate, or wrap them in a custom checked or unchecked exception.

-
The behaviour of the code doesn't match your description in one respect. You talk about queuing messages. In fact, messages are being sent immediately in all cases, and you are delaying after sending the message in the sendRawLine case.

-
In reality, you are blocking client threads rather using a queue data structure and a separate output thread. This might be the right things to do ... but it might be the wrong thing. It depends whether the client threads should be blocked until the their respective messages are sent. Either way, your characterization this as a "message queuing" problem is a bit misleading.


Re: #3) The idea was to delay the next message from sending. Otherwise all calls to sendRawLine would have a weird pause before doing anything. I'm looking into using the current time though to determine if waiting is even necessary.

First, that is NOT what your description of "message queuing" implies ... to me. And consider me as an examplar of someone else reading your code.

Second, (if I understand you correctly) using the current time to decide to whether to wait before a send would require that something keeps track of when each thread last sent a message.

Third, there is actually a good reason to not wait before sending a message. If you do wait before then, by the time the wait period has finished, the message could be rather out of date ... especially if there were a number of resets.

Finally, it strikes me that you may be over-using threads here and/or that it might be better for the entity (thread or FSM) responsible for generating messages should check whether a message needs to be sent before generating it.

StackExchange Code Review Q#27567, answer score: 3