std.concurrency: receiveTimeout: allow negative timeout

[sigod]

Related #3516.

This assert was introduced in #1910, which contradicts #1279.

[DmitryOlshansky]

LGTm

[WalterBright]

Rationale?

[sigod]

@complexmath, you might like to take a look at this.

[complexmath]

A negative timeout doesn't really make sense. I'd vote in favor of requiring a non-negative timeout. Or alternately, considering any negative timeout equivalent to zero if we want to be forgiving of bad math. As long as it's consistent, either is acceptable, though rejecting negative values makes a bit more sense to me.

[sigod]

A negative timeout doesn't really make sense.

Indeed it doesn't. But I hoped to hear some details of why you worked with negative timeout originally.

I agree with requiring a non-negative timeout. But let's make zero timeout a true no wait. As it's currently with negative values:

    if( period.isNegative || !m_putMsg.wait( period ) )
        return false;

Shall I create new PR?

[schveiguy]

Or alternately, considering any negative timeout equivalent to zero if we want to be forgiving of bad math

I think this is the best option. Timing on a non-realtime OS is never exact, and if you are running a calculation of how long to wait for something, it's quite easy to get into this situation.

For example if you want to check for messages while running a callback every 100ms, it's quite possible your callback may take longer than 100ms (or you get swapped out by the OS), and then your next wait time will be negative. To push that check onto the user seems counter-productive.

[complexmath]

@sigod No. The current behavior for a timeout of zero is equivalent to a yield(), which I think is important if the underlying scheduler incorporates fibers.

[complexmath]

@schveiguy That was the case I considered as well. It seems like it could be easy to wind up with a negative timeout if you're decaying the value or something. If I were writing the user code I'd check, but I think it's fair to state that the API will handle these values gracefully instead of asserting.

[DmitryOlshansky]

I agree with requiring a non-negative timeout. But let's make zero timeout a true no wait. As it's currently with negative values:

if( period.isNegative || !m_putMsg.wait( period ) )
    return false;

Shall I create new PR?

Seems like both should 'yield' current thread/fiber. @sigod up to the task?

[sigod]

@DmitryOlshansky Sorry for long response.

Seems like both should 'yield' current thread/fiber.

You mean that both, zero and negative timeouts, should "yield"?

Like this?

if( period <= Duration.zero || !m_putMsg.wait( period ) )
    return false;

[DmitryOlshansky]

You mean that both, zero and negative timeouts, should "yield"?

Like this?

Yes - that would check mailbox and yeild if empty.

[sigod]

Done.

[sigod]

@DmitryOlshansky, ping.

[DmitryOlshansky]

Auto-merge toggled on

[DmitryOlshansky]

@sigod Sorry forgot about this one