Bug 224615 - [PATCH] kevent: EVFILT_READ returns EV_EOF on named pipe when it should not
Summary: [PATCH] kevent: EVFILT_READ returns EV_EOF on named pipe when it should not
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Johnston
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-12-27 14:16 UTC by Jan Kokemüller
Modified: 2020-05-11 15:22 UTC (History)
5 users (show)

See Also:


Attachments
patch for filt_piperead (894 bytes, patch)
2017-12-27 14:16 UTC, Jan Kokemüller
no flags Details | Diff
patch to pipepoll.c demonstrating the issue (1.21 KB, patch)
2017-12-27 14:16 UTC, Jan Kokemüller
no flags Details | Diff
Updated FIFO/pipe fixes (6.14 KB, patch)
2020-04-22 01:44 UTC, Jan Kokemüller
no flags Details | Diff
Updated FIFO/pipe fixes (more context) (57.24 KB, patch)
2020-04-22 01:48 UTC, Jan Kokemüller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kokemüller 2017-12-27 14:16:13 UTC
Created attachment 189130 [details]
patch for filt_piperead

When connecting as a new reader to a named pipe after all previous writers have disconnected, EVFILT_READ currently returns EV_EOF.

poll(2) was fixed a while ago so that it won't return POLLHUP in this case (at least since r238936). The attached patch brings the logic in EVFILT_READ in sync with poll(2) again.

I wonder why the other side of the pipe is also checked in EVFILT_READ. Wouldn't it be enough to just check for EOF on the readers side? This would be consistent with sockets for example (where just SBS_CANTRCVMORE leads to EV_EOF).

I've also attached a small patch to the pipepoll.c regression test that can be used to reproduce the issue.
Comment 1 Jan Kokemüller 2017-12-27 14:16:53 UTC
Created attachment 189131 [details]
patch to pipepoll.c demonstrating the issue
Comment 2 Jan Kokemüller 2020-04-22 01:44:44 UTC
Created attachment 213659 [details]
Updated FIFO/pipe fixes

I've been running these FIFO/pipe fixes for some time now. I've attached the current version of the patch. This should also fix https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=203366 and some other minor issues I've come across.

I wrote a lot of ATF tests for FIFO/pipe behavior with kqueue/poll here: https://github.com/jiixyj/epoll-shim/blob/develop/test/pipe-test.c . I'd love to clean them up for inclusion into FreeBSD. Currently there is lots of code to test FIFOs/pipes under other operating systems as well which obscures the important stuff a bit.
Comment 3 Jan Kokemüller 2020-04-22 01:48:34 UTC
Created attachment 213662 [details]
Updated FIFO/pipe fixes (more context)

I've regenerated the patch with more context (git show -U999999).
Comment 4 Kyle Evans freebsd_committer 2020-04-22 01:51:31 UTC
(In reply to Jan Kokemüller from comment #3)

Ah, excellent, thanks! =-) Any chance you have a Phabricator account and would be willing to upload it there, as well, so we can loop in some other folks?
Comment 5 Jan Kokemüller 2020-04-22 02:54:04 UTC
Sure thing, I've opened https://reviews.freebsd.org/D24528 !
Comment 6 commit-hook freebsd_committer 2020-04-27 15:59:51 UTC
A commit references this bug:

Author: markj
Date: Mon Apr 27 15:59:20 UTC 2020
New revision: 360380
URL: https://svnweb.freebsd.org/changeset/base/360380

Log:
  Fix handling of EV_EOF for named pipes.

  Contrary to the kevent man page, EV_EOF on a fifo is not cleared by
  EV_CLEAR.  Modify the read and write filters to clear EV_EOF when the
  fifo's PIPE_EOF flag is clear, and update the man page to document the
  new behaviour.

  Modify the write filter to return the amount of buffer space available
  even if no readers are present.  This matches the behaviour for sockets.

  When reading from a pipe, only call pipeselwakeup() if some data was
  actually read.  This prevents the continuous re-triggering of a
  EVFILT_READ event on EOF when in edge-triggered mode.

  PR:		203366, 224615
  Submitted by:	Jan Kokem?ller <jan.kokemueller@gmail.com>
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D24528

Changes:
  head/lib/libc/sys/kqueue.2
  head/sys/kern/sys_pipe.c
Comment 7 commit-hook freebsd_committer 2020-05-11 15:20:49 UTC
A commit references this bug:

Author: markj
Date: Mon May 11 15:20:41 UTC 2020
New revision: 360897
URL: https://svnweb.freebsd.org/changeset/base/360897

Log:
  MFC r360380:
  Fix handling of EV_EOF for named pipes.

  PR:	203366, 224615, 246350

Changes:
_U  stable/12/
  stable/12/lib/libc/sys/kqueue.2
  stable/12/sys/kern/sys_pipe.c