This doesn't fix the problem. There is still a race if the timeout callback is triggered directly after msg_receive(); in _msg_wait but before xtimer_remove(t);. I'll update the code snippet in my other PR to show the problem.

There is still a race if the timeout callback is triggered directly after msg_receive(); in _msg_wait but before xtimer_remove(t);

Why?

This fix ensures that there are no queued messages (function doesn't pass the early out). Now, if any received message was not a timeout message, the xtimer is removed.

In case that removed xtimer has sent a message in between (after a "real" message, and with a queue present), that could now still be queued. But the early out prevents the function from misinterpreting that message as its own (which lead to the initial issue).

I'll update the code snippet in my other PR to show the problem.

Which one? I'm getting confused. :)

Now, if any received message was not a timeout message, the xtimer is removed.

Yes, but directly before the remove is executed, the timeout callback may still trigger.
So when receiving a proper msg after msg_receive(m);, but before the xtimer could be removed.
Basically directly before this line.

Which one? I'm getting confused. :)

#13500 (the collapsed code snippet)

Yes, but directly before the remove is executed, the timeout callback may still trigger.
So when receiving a proper msg after msg_receive(m);, but before the xtimer could be removed.

If another "proper" message is received, no harm done. It'll queue (or let the sender wait).
If xtimer triggers before it's "remove", the remove will run empty, but that is ok.

Worst case is that the message from that timer could get queued.

It's not about another proper message but indeed the timeout message.
One could argue that it is okay to enqueue the timeout message but I think this is nowhere in the documentation and IMO not to be preferred.

But wouldn't it be much more consequent to just always return a timeout message on timeout? With a timeout message waiting in the queue the timing behavior of following calls gets very "interesting" ;)

With a timeout message waiting in the queue the timing behavior of following calls gets very "interesting" ;)

It would receive the timeout message, but not return the timeout return value.
Hopefully, all message handlers are written like:

switch(msg->type):
case EXPECTED:
//...
default:
  DEBUG("unexpected message type %i\n", msg->type);

But wouldn't it be much more consequent to just always return a timeout message on timeout?

You mean when a "proper" message has been received bit also the timeout triggered?
This is difficult to model without a messge queue, as the "proper" message needs to go somewhere. Also, technically, the message was received before the timeout.

It would receive the timeout message, but not return the timeout return value.

No. First, it would return 1, indicating a message was received. In the same step the timeout message is added to the queue. On the next call it will immediately return the timeout message without waiting for a timeout.

You mean when a "proper" message has been received bit also the timeout triggered?

I meant that if we use the returned message to signal a timeout, this should be always used for indicating a timeout and not either the return value or the timeout message indicates a timeout.

No. First, it would return 1, indicating a message was received. In the same step the timeout message is added to the queue. On the next call it will immediately return the timeout message without waiting for a timeout.

Yes, but in the second call, it would return "1", indicating that a message was received, vs. returning "-1". The received message would be the timeout message, though.

I meant that if we use the returned message to signal a timeout,

Ah, I didn't get that if. Why change the API? ALso, doesn't solve the problem, as then the former queued message would be returned.

The received message would be the timeout message, though.

Yes, but it will return the timeout message immediately, that s the problem. And also it will return the timeout message for another call of msg_recv_timeout() i.e. when another fresh timeout should actually start.

Why change the API?

I don't want to^^ but using the message as timeout indication would be just that...

ALso, doesn't solve the problem, as then the former queued message would be returned.

So you agree it doesn't work as is, right?