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

[Pavel Begunkov <asml.silence@gmail.com>]

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.

diff --git a/io_uring/poll.c b/io_uring/poll.c
index b671b84657d9..3944deb55234 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -43,7 +43,8 @@ struct io_poll_table {
  
  #define IO_POLL_CANCEL_FLAG	BIT(31)
  #define IO_POLL_RETRY_FLAG	BIT(30)
-#define IO_POLL_REF_MASK	GENMASK(29, 0)
+#define IO_POLL_HUP_FLAG	BIT(29)
+#define IO_POLL_REF_MASK	GENMASK(28, 0)
  
  /*
   * We usually have 1-2 refs taken, 128 is more than enough and we want to
@@ -272,6 +273,8 @@ static int io_poll_check_events(struct io_kiocb *req, io_tw_token_t tw)
  				atomic_andnot(IO_POLL_RETRY_FLAG, &req->poll_refs);
  				v &= ~IO_POLL_RETRY_FLAG;
  			}
+			if (v & IO_POLL_HUP_FLAG)
+				atomic_or(IO_POLL_RETRY_FLAG, &req->poll_refs);
  		}
  
  		/* the mask was stashed in __io_poll_execute */
@@ -390,6 +393,14 @@ static __cold int io_pollfree_wake(struct io_kiocb *req, struct io_poll *poll)
  	return 1;
  }
  
+static void io_handle_hup(struct io_kiocb *req, struct io_poll *poll)
+{
+	if (req->opcode == IORING_OP_POLL_ADD)
+		return;
+	if (poll->events & (POLLIN|EPOLLRDNORM))
+		atomic_or(IO_POLL_HUP_FLAG, &req->poll_refs);
+}
+
  static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
  			void *key)
  {
@@ -397,8 +408,12 @@ static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
  	struct io_poll *poll = container_of(wait, struct io_poll, wait);
  	__poll_t mask = key_to_poll(key);
  
-	if (unlikely(mask & POLLFREE))
-		return io_pollfree_wake(req, poll);
+	if (unlikely(mask & (POLLFREE|POLLHUP))) {
+		if (mask & POLLFREE)
+			return io_pollfree_wake(req, poll);
+		if (mask & POLLHUP)
+			io_handle_hup(req, poll);
+	}
  
  	/* for instances that support it check for an event match first */
  	if (mask && !(mask & (poll->events & ~IO_ASYNC_POLL_COMMON)))

-- 
Pavel Begunkov