On 29/10/21(Fri) 14:47, Martin Pieuchot wrote:
> On 29/10/21(Fri) 10:50, Anton Lindqvist wrote:
> > On Fri, Oct 29, 2021 at 09:13:58AM +0100, Larry Hynes wrote:
> > > Hi
> > >
> > > In last two snapshots I installed, poll seems to hang in certain
> > > circumstances.
> > >
> > > Here's a reproducer, from Leah Neukirchen (cc'ed on this mail):
>
> Thanks, do you agree to license it under ISC, as in
> /usr/share/misc/license.template, so we can put it in src/regress?
>
> > >
> > > ------
> > >
> > > #include <stdio.h>
> > > #include <poll.h>
> > > #include <unistd.h>
> > >
> > > int
> > > main()
> > > {
> > > struct pollfd fds[1];
> > >
> > > fds[0].fd = 0;
> > > fds[0].events = POLLIN | POLLHUP;
> > > close(0);
> > >
> > > printf("%d\n", poll(fds, 1, -1));
> > > printf("%d\n", fds[0].revents & POLLNVAL);
> > > }
> > >
> > > ------
> > >
> > > When compiled and run, that should hang on latest snap. It would be
> > > expected to return immediately with POLLNVAL set.
> >
> > I think poll() needs similar handling as select() recently gained.
> > Caution, only compile tested.
>
> Here's a simpler fix, do not sleep at all if we already have something
> to report.
After some discussions with anton@ I believe we should instead go with a
version similar to what sys_select is doing. It is necessary to call
ppollcollect() for FDs that are ready.
Here's anton@'s version without extra argument:
Index: kern/sys_generic.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.138
diff -u -p -r1.138 sys_generic.c
--- kern/sys_generic.c 24 Oct 2021 11:23:22 -0000 1.138
+++ kern/sys_generic.c 29 Oct 2021 14:05:56 -0000
@@ -1128,7 +1128,7 @@ doppoll(struct proc *p, struct pollfd *f
{
struct kqueue_scan_state scan;
struct pollfd pfds[4], *pl = pfds;
- int error, nevents = 0;
+ int error, ncollected, nevents = 0;
size_t sz;
/* Standards say no more than MAX_OPEN; this is possibly better. */
@@ -1154,14 +1154,14 @@ doppoll(struct proc *p, struct pollfd *f
dosigsuspend(p, *sigmask &~ sigcantmask);
/* Register kqueue events */
- *retval = ppollregister(p, pl, nfds, &nevents);
+ ncollected = ppollregister(p, pl, nfds, &nevents);
/*
* The poll/select family of syscalls has been designed to
* block when file descriptors are not available, even if
* there's nothing to wait for.
*/
- if (nevents == 0) {
+ if (nevents == 0 && ncollected == 0) {
uint64_t nsecs = INFSLP;
if (timeout != NULL) {
@@ -1184,7 +1184,7 @@ doppoll(struct proc *p, struct pollfd *f
struct kevent kev[KQ_NEVENTS];
int i, ready, count;
- /* Maxium number of events per iteration */
+ /* Maximum number of events per iteration */
count = MIN(nitems(kev), nevents);
ready = kqueue_scan(&scan, count, kev, timeout, p, &error);
#ifdef KTRACE
@@ -1193,7 +1193,7 @@ doppoll(struct proc *p, struct pollfd *f
#endif
/* Convert back events that are ready. */
for (i = 0; i < ready; i++)
- *retval += ppollcollect(p, &kev[i], pl, nfds);
+ ncollected += ppollcollect(p, &kev[i], pl, nfds);
/*
* Stop if there was an error or if we had enough
@@ -1205,6 +1205,7 @@ doppoll(struct proc *p, struct pollfd *f
nevents -= ready;
}
kqueue_scan_finish(&scan);
+ *retval = ncollected;
done:
/*
* NOTE: poll(2) is not restarted after a signal and EWOULDBLOCK is
