From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN:  
X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no
	autolearn=ham autolearn_force=no version=3.4.6
Received: from localhost (dcvr.yhbt.net [127.0.0.1])
	by dcvr.yhbt.net (Postfix) with ESMTP id 4AD331F406
	for <meta@public-inbox.org>; Mon, 30 Oct 2023 18:31:21 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org;
	s=selector1; t=1698690681;
	bh=lYF7dh/4b9dYV0sjxc39B2WbLirxatkw641IsYArmbE=;
	h=From:To:Subject:Date:From;
	b=m057mNke3C0GvfmR14ywxjZlVneTe0fgKThBbuiHSmHQZQeozRHFv9Ah12FNEI15F
	 pMNgn9ArKAsOo5YMyg9NPId/Oh8VX1n1886JPJi34VM4LdAXz7TBU3l19fclDzjnkg
	 ua2n6R7fMSUKL4DByT/OXT6JhBZPeCwJU57hFIaA=
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH] poll+select: check EBADF + POLLNVAL errors
Date: Mon, 30 Oct 2023 18:29:40 +0000
Message-ID: <20231030182940.5628-1-e@80x24.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
List-Id: <meta.public-inbox.org>

I hit this in via select running -cindex with some other
experimental patches.  I can't reproduce the problem, though,
but this ensure we have a chance to diagnose it if it happens
again instead of looping on select(2) => EBADF.
---
 lib/PublicInbox/DSPoll.pm | 31 ++++++++++++++++++++-----------
 lib/PublicInbox/Select.pm |  3 ++-
 t/ds-poll.t               | 21 +++++++++++++++++++--
 3 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/DSPoll.pm b/lib/PublicInbox/DSPoll.pm
index fc282de0..0446df4c 100644
--- a/lib/PublicInbox/DSPoll.pm
+++ b/lib/PublicInbox/DSPoll.pm
@@ -12,30 +12,39 @@ package PublicInbox::DSPoll;
 use v5.12;
 use IO::Poll;
 use PublicInbox::Syscall qw(EPOLLONESHOT EPOLLIN EPOLLOUT);
+use Carp qw(carp);
+use Errno ();
 
 sub new { bless {}, __PACKAGE__ } # fd => events
 
 sub ep_wait {
 	my ($self, $maxevents, $timeout_msec, $events) = @_;
-	my @pset;
+	my (@pset, $n, $fd, $revents, $nval);
 	while (my ($fd, $events) = each %$self) {
 		my $pevents = $events & EPOLLIN ? POLLIN : 0;
 		$pevents |= $events & EPOLLOUT ? POLLOUT : 0;
 		push(@pset, $fd, $pevents);
 	}
 	@$events = ();
-	my $n = IO::Poll::_poll($timeout_msec, @pset);
-	if ($n >= 0) {
-		for (my $i = 0; $i < @pset; ) {
-			my $fd = $pset[$i++];
-			my $revents = $pset[$i++] or next;
-			delete($self->{$fd}) if $self->{$fd} & EPOLLONESHOT;
+	do {
+		$n = IO::Poll::_poll($timeout_msec, @pset);
+	} while ($n < 0 && $! == Errno::EINTR);
+	die "poll: $!" if $n < 0;
+	return if $n == 0;
+	while (defined($fd = shift @pset)) {
+		$revents = shift @pset or next; # no event
+		if ($revents & POLLNVAL) {
+			carp "E: FD=$fd invalid in poll";
+			delete $self->{$fd};
+			$nval = 1;
+		} else {
+			delete $self->{$fd} if $self->{$fd} & EPOLLONESHOT;
 			push @$events, $fd;
 		}
-		my $nevents = scalar @$events;
-		if ($n != $nevents) {
-			warn "BUG? poll() returned $n, but got $nevents";
-		}
+	}
+	if ($nval && !@$events) {
+		$! = Errno::EBADF;
+		die "poll: $!";
 	}
 }
 
diff --git a/lib/PublicInbox/Select.pm b/lib/PublicInbox/Select.pm
index 9df3a6bd..5817c9ef 100644
--- a/lib/PublicInbox/Select.pm
+++ b/lib/PublicInbox/Select.pm
@@ -20,7 +20,8 @@ sub ep_wait {
 	}
 	@$events = ();
 	my $n = select($rvec, $wvec, undef, $msec < 0 ? undef : ($msec/1000));
-	return if $n <= 0;
+	return if $n == 0;
+	die "select: $!" if $n < 0;
 	while (my ($fd, $ev) = each %$self) {
 		if (vec($rvec, $fd, 1) || vec($wvec, $fd, 1)) {
 			delete($self->{$fd}) if $ev & EPOLLONESHOT;
diff --git a/t/ds-poll.t b/t/ds-poll.t
index 57fac3ef..7a7e2bcf 100644
--- a/t/ds-poll.t
+++ b/t/ds-poll.t
@@ -6,13 +6,14 @@
 use v5.12;
 use Test::More;
 use PublicInbox::Syscall qw(EPOLLIN EPOLLOUT EPOLLONESHOT);
+use autodie qw(close pipe syswrite);
 my $cls = $ENV{TEST_IOPOLLER} // 'PublicInbox::DSPoll';
 use_ok $cls;
 my $p = $cls->new;
 
 my ($r, $w, $x, $y);
-pipe($r, $w) or die;
-pipe($x, $y) or die;
+pipe($r, $w);
+pipe($x, $y);
 is($p->ep_add($r, EPOLLIN), 0, 'add EPOLLIN');
 my $events = [];
 $p->ep_wait(9, 0, $events);
@@ -44,4 +45,20 @@ is($p->ep_del($r, 0), 0, 'EPOLL_CTL_DEL OK');
 $p->ep_wait(9, 0, $events);
 is(scalar @$events, 0, 'nothing ready after EPOLL_CTL_DEL');
 
+is($p->ep_add($r, EPOLLIN), 0, 're-add');
+SKIP: {
+	$cls =~ m!::(?:DSPoll|Select)\z! or
+		skip 'EBADF test for select|poll only';
+	my $old_fd = fileno($r);
+	close $r;
+	my @w;
+	eval {
+		local $SIG{__WARN__} = sub { push @w, @_ };
+		$p->ep_wait(9, 0, $events);
+	};
+	ok($@, 'error detected from bad FD');
+	ok($!{EBADF}, 'EBADF errno set');
+	@w and ok(grep(/\bFD=$old_fd invalid/, @w), 'carps invalid FD');
+}
+
 done_testing;