Re: kern/58929: POSIX.1-2024 compliance: posix_close, POSIX_CLOSE_RESTART

["Robert Elz via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/58929; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/58929: POSIX.1-2024 compliance: posix_close, POSIX_CLOSE_RESTART
Date: Sun, 22 Dec 2024 07:49:25 +0700

     Date:        Sat, 21 Dec 2024 22:07:19 +0000
     From:        Taylor R Campbell <riastradh%NetBSD.org@localhost>
     Message-ID:  <20241221220723.04ECB84E92%mail.netbsd.org@localhost>
 
   | Correction: Our current close() -- and probably all existing close()
   | implementations outside buggy proprietary Unix variants -- is not
   | compliant with POSIX.1-2024, because if some underlying I/O is
   | interrupted by a signal it can fail with EINTR even though the
   | descriptor is unconditionally closed.  That is now forbidden (emphasis
   | added):
 
 Can it really?   I haven't looked at our implementation (kernel)
 but it used to be the case that even if close() slept for some time,
 (like waiting for tty output to drain) and a signal occurred, close()
 would still return 0.
 
 But if not, surely the simple change here is to make that happen, not
 to work around it in libc - just have sys_close() never return EINTR.
 
 We have to function correctly in this case anyway, as we do close the
 fd, so another close(fd) will just return EBADF - so anything draining
 still in the kernel output queues needs to be properly handled without
 any more userland assistance.
 
 kre
   |
   | > If close() is interrupted by a signal that is to be caught, then it
   | > is unspecified whether it returns -1 with errno set to [EINTR] and
   | > fildes _remaining open_, or returns -1 with errno set to
   | > [EINPROGRESS] and fildes being closed, or returns 0 to indicate
   | > successful completion[...]
   | > 
   | > RATIONALE
   | > 
   | > [...] Note that the standard requires that close() and posix_close()
   | > _must leave fildes open after [EINTR]_ (in the cases where [EINTR]
   | > is permitted)
   |
   | So, in order to deal with POSIX.1-2024 having addressed the problem of
   | its own failure to adequately specify semantics by breaking
   | compatibility with every major OS on the planet, it looks like we'll
   | have to do some symbol magic under _POSIX_C_SOURCE -- something like
   | this:
   |
   | /* include/unistd.h */
   |
   | #if (_POSIX_C_SOURCE >= 202408L) && !defined(_NETBSD_SOURCE)
   | int	close(int) __RENAME(__posixly_correct_close);
   | int	posix_close(int, int);
   | #define	POSIX_CLOSE_RESTART	0
   | #else
   | int	close(int);
   | #endif
   |
   | /* lib/libc/sys/posixly_correct_close.c */
   |
   | int
   | __posixly_correct_close(int d)
   | {
   | 	const int ret = close(d);
   |
   | 	if (ret == -1 && errno == EINTR)
   | 		errno = EINPROGRESS;
   |
   | 	return ret;
   | }
   |
   | int
   | posix_close(int d, int flag)
   | {
   | 	const int ret = __posixly_correct_close(d);
   |
   | 	_DIAGASSERT(ret == 0 || errno != EINTR);
   |
   | 	if (ret == 0 && flag != 0) {
   | 		errno = EINVAL;
   | 		ret = -1;
   | 	}
   | 	return ret;
   | }
   |
   | What a colossal waste of effort -- and attack surface for new bugs to
   | crawl onto -- to force everyone else to bend over backwards to
   | accommodate bugs in proprietary implementations of close().
   |