test: handle zero-length udp datagram #4344
Conversation
test/test-udp-recv-in-a-row.c
Outdated
| * | ||
| * See https://github.com/libuv/libuv/issues/4219. | ||
| */ | ||
| ASSERT_GE(nread, 0); |
There was a problem hiding this comment.
I think it would be better to be exact here:
| ASSERT_GE(nread, 0); | |
| if (nread) ASSERT_EQ(nread, sizeof(send_data)); |
Under rare but benign circumstances, incoming datagrams are dropped by the operating system after libuv has been notified of their arrival but before libuv has had a chance to receive them. Fixes: libuv#4219
| @@ -63,9 +68,11 @@ static void check_cb(uv_check_t* handle) { | |||
| /** | |||
| * sv_recv_cb() is called with nread set to zero to indicate | |||
| * there is no more udp packet in the kernel, so the actual | |||
| * recv_cnt is one larger than N. | |||
| * recv_cnt is up to one larger than N. UDP being what it is, | |||
| * packets can get dropped so don't assume an exact count. | |||
There was a problem hiding this comment.
@vtjnash after looking at the code some more, I came the conclusion that the "is equal to N+1" check is wrong because any number of packets can get dropped. Even assuming a single packet makes it through is already a shaky assumption, just one that is true most of the time.
There was a problem hiding this comment.
IIUC, the kernel does end up guaranteeing reliable UDP delivery to localhost. It is sort of by-accident, but it would be more work for it to discard messages because it goes through loopback directly instead of the making a whole roundtrip through the hardware.
There was a problem hiding this comment.
Linux maybe but XNU? The CI failures are on macos buildbots.
FWIW, I can sort of reproduce it on Linux with a traffic-shaped device but at that point it's not really localhost traffic anymore so maybe not completely apples to apples.
Under rare but benign circumstances, incoming datagrams are dropped by the operating system after libuv has been notified of their arrival but before it has had a chance to receive them.
Fixes: #4219