Re: [PATCH] io_uring/poll: fix multishot recv missing EOF on wakeup race

[Jens Axboe <axboe@kernel.dk>]

On 3/16/26 9:16 AM, Jens Axboe wrote:
> On 3/16/26 8:44 AM, Pavel Begunkov wrote:
>> On 3/16/26 14:40, Pavel Begunkov wrote:
>>> On 3/16/26 14:28, Jens Axboe wrote:
>>>> On 3/16/26 8:17 AM, Pavel Begunkov wrote:
>>>>> On 3/15/26 16:19, Jens Axboe wrote:
>>>>>> When a socket send and shutdown() happen back-to-back, both fire
>>>>>> wake-ups before the receiver's task_work has a chance to run. The first
>>>>>> wake gets poll ownership (poll_refs=1), and the second bumps it to 2.
>>>>>> When io_poll_check_events() runs, it calls io_poll_issue() which does a
>>>>>> recv that reads the data and returns IOU_RETRY. The loop then drains all
>>>>>> accumulated refs (atomic_sub_return(2) -> 0) and exits, even though only
>>>>>> the first event was consumed. Since the shutdown is a persistent state
>>>>>> change, no further wakeups will happen, and the multishot recv can hang
>>>>>> forever.
>>>>>>
>>>>>> Fix this by only draining a single poll ref after io_poll_issue()
>>>>>> returns IOU_RETRY for the APOLL_MULTISHOT path. If additional wakes
>>>>>> raced in (poll_refs was > 1), the loop iterates again, vfs_poll()
>>>>>> discovers the remaining state.
>>>>>
>>>>> How often will iterate with no effect for normal execution (i.e.
>>>>> no shutdown)? And how costly it'll be? Why not handle HUP instead?
>>>>
>>>> That is my worry too. I spent a bit of time on it this morning to figure
>>>> out why this is a new issue, and traced it down to 6.16..6.17, and this
>>>> commit in particular:
>>>>
>>>> commit df30285b3670bf52e1e5512e4d4482bec5e93c16
>>>> Author: Kuniyuki Iwashima <kuniyu@google.com>
>>>> Date:   Wed Jul 2 22:35:18 2025 +0000
>>>>
>>>>      af_unix: Introduce SO_INQ.
>>>>
>>>> which is then not the first time I've had to fix fallout from that
>>>> commit. Need to dig a bit deeper. That said, I do also worry a bit about
>>>> missing events. Yes if both poll triggers are of the same type, eg
>>>> POLLIN, then we don't need to iterate again. IN + HUP is problematic, as
>>>> would anything else where you'd need separate handling for the trigger.
>>>
>>> Thinking more, I don't think the patch is correct either. Seems you
>>> expect the last recv to return 0, but let's say you have 2 refs and
>>> 8K in the rx queue. The first recv call gets 4K b/c some allocation
>>> fails. The 2nd recv call returns another 4K, and now you're in the
>>> same situation as before.
>>>
>>> You're trying to rely on a too specific behaviour. HUP handling should
>>> be better.
>>
>> Some variation on, if HUP'ed, it spins until the opcode give up.
> 
> Took a quick look, and we don't even get a HUP, the hangup side
> ends up with a 0 mask. Which is less than useful... I'll keep
> digging.

How about something like this? Will only retry if hup was seen, and
there are multiple refs. Avoids re-iterating for eg multiple POLLIN
wakes, which should be the common hot path if v & IO_POLL_REF_MASK != 1.
Keeps it local too.

diff --git a/io_uring/poll.c b/io_uring/poll.c
index b671b84657d9..bd79a04a2c59 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -228,6 +228,16 @@ static inline void io_poll_execute(struct io_kiocb *req, int res)
 		__io_poll_execute(req, res);
 }
 
+static inline bool io_poll_loop_retry(struct io_kiocb *req, int v)
+{
+	if (req->opcode == IORING_OP_POLL_ADD)
+		return false;
+	/* multiple refs and HUP, ensure we loop once more */
+	if (v != 1 && req->cqe.res & (POLLHUP | POLLRDHUP))
+		return true;
+	return false;
+}
+
 /*
  * All poll tw should go through this. Checks for poll events, manages
  * references, does rewait, etc.
@@ -246,8 +256,9 @@ static int io_poll_check_events(struct io_kiocb *req, io_tw_token_t tw)
 		return -ECANCELED;
 
 	do {
-		v = atomic_read(&req->poll_refs);
+		bool retry = false;
 
+		v = atomic_read(&req->poll_refs);
 		if (unlikely(v != 1)) {
 			/* tw should be the owner and so have some refs */
 			if (WARN_ON_ONCE(!(v & IO_POLL_REF_MASK)))
@@ -287,13 +298,15 @@ static int io_poll_check_events(struct io_kiocb *req, io_tw_token_t tw)
 			if (unlikely(!req->cqe.res)) {
 				/* Multishot armed need not reissue */
 				if (!(req->apoll_events & EPOLLONESHOT))
-					continue;
+					goto finish;
 				return IOU_POLL_REISSUE;
 			}
 		}
 		if (req->apoll_events & EPOLLONESHOT)
 			return IOU_POLL_DONE;
 
+		retry = io_poll_loop_retry(req, v);
+
 		/* multishot, just fill a CQE and proceed */
 		if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
 			__poll_t mask = mangle_poll(req->cqe.res &
@@ -317,12 +330,17 @@ static int io_poll_check_events(struct io_kiocb *req, io_tw_token_t tw)
 		/* force the next iteration to vfs_poll() */
 		req->cqe.res = 0;
 
+		if (retry)
+			continue;
 		/*
 		 * Release all references, retry if someone tried to restart
 		 * task_work while we were executing it.
 		 */
+finish:
 		v &= IO_POLL_REF_MASK;
-	} while (atomic_sub_return(v, &req->poll_refs) & IO_POLL_REF_MASK);
+		if (!(atomic_sub_return(v, &req->poll_refs) & IO_POLL_REF_MASK))
+			break;
+	} while (1);
 
 	io_napi_add(req);
 	return IOU_POLL_NO_ACTION;

-- 
Jens Axboe