Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Apr 2015 10:59:33 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        arch@freebsd.org
Subject:   Make ppoll(2) and waitid(2) cancellation points
Message-ID:  <20150417075933.GH2390@kib.kiev.ua>

next in thread | raw e-mail | index | archive | help
Patch below makes ppoll(2) and waitid(2) cancellable.

The waitid(2) is required to be a cancellation point by POSIX, so the
change fixes a definite bug. It is done by making wait6() cancellable,
in line with the other wait*(2) syscalls.

For ppoll(2), our other select/poll interfaces are cancel points, i.e.
select, pselect, and poll. It is reasonable for ppoll() to follow the
suite.

The interposing table was extended at the end, instead of ordering new
interposers by alphabet.  It would be acceptable, but makes it easier
for people to run new libc with older libthr (although not supported).

diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_private.h
index ceaa2a3..2de6f77 100644
--- a/lib/libc/include/libc_private.h
+++ b/lib/libc/include/libc_private.h
@@ -222,6 +222,8 @@ enum {
 	INTERPOS_spinlock,
 	INTERPOS_spinunlock,
 	INTERPOS_kevent,
+	INTERPOS_wait6,
+	INTERPOS_ppoll,
 	INTERPOS_MAX
 };
 
@@ -305,6 +307,8 @@ struct timeval;
 struct timezone;
 struct __siginfo;
 struct __ucontext;
+struct __wrusage;
+enum idtype;
 int		__sys_aio_suspend(const struct aiocb * const[], int,
 		    const struct timespec *);
 int		__sys_accept(int, struct sockaddr *, __socklen_t *);
@@ -329,6 +333,8 @@ int		__sys_pselect(int, struct fd_set *, struct fd_set *,
 		    struct fd_set *, const struct timespec *,
 		    const __sigset_t *);
 int		__sys_poll(struct pollfd *, unsigned, int);
+int		__sys_ppoll(struct pollfd *, unsigned, const struct timespec *,
+		    const __sigset_t *);
 __ssize_t	__sys_pread(int, void *, __size_t, __off_t);
 __ssize_t	__sys_pwrite(int, const void *, __size_t, __off_t);
 __ssize_t	__sys_read(int, void *, __size_t);
@@ -357,6 +363,8 @@ int		__sys_thr_kill(long, int);
 int		__sys_thr_self(long *);
 int		__sys_truncate(const char *, __off_t);
 __pid_t		__sys_wait4(__pid_t, int *, int, struct rusage *);
+__pid_t		__sys_wait6(enum idtype, __id_t, int *, int,
+		    struct __wrusage *, struct __siginfo *);
 __ssize_t	__sys_write(int, const void *, __size_t);
 __ssize_t	__sys_writev(int, const struct iovec *, int);
 
diff --git a/lib/libc/sys/Makefile.inc b/lib/libc/sys/Makefile.inc
index 7745b2a..a38a923 100644
--- a/lib/libc/sys/Makefile.inc
+++ b/lib/libc/sys/Makefile.inc
@@ -57,6 +57,7 @@ INTERPOSED = \
 	open \
 	openat \
 	poll \
+	ppoll \
 	pselect \
 	read \
 	readv \
@@ -73,6 +74,7 @@ INTERPOSED = \
 	sigwaitinfo \
 	swapcontext \
 	wait4 \
+	wait6 \
 	write \
 	writev
 
diff --git a/lib/libc/sys/interposing_table.c b/lib/libc/sys/interposing_table.c
index 4290bc6..c4d1429 100644
--- a/lib/libc/sys/interposing_table.c
+++ b/lib/libc/sys/interposing_table.c
@@ -76,6 +76,8 @@ interpos_func_t __libc_interposing[INTERPOS_MAX] = {
 	SLOT(spinlock, __libc_spinlock_stub),
 	SLOT(spinunlock, __libc_spinunlock_stub),
 	SLOT(kevent, __sys_kevent),
+	SLOT(wait6, __sys_wait6),
+	SLOT(ppoll, __sys_ppoll),
 };
 #undef SLOT
 
diff --git a/lib/libc/sys/ppoll.c b/lib/libc/sys/ppoll.c
new file mode 100644
index 0000000..f62fd19
--- /dev/null
+++ b/lib/libc/sys/ppoll.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2015 The FreeBSD Foundation.
+ * All rights reserved.
+ *
+ * Portions of this software were developed by Konstantin Belousov
+ * under sponsorship from the FreeBSD Foundation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice(s), this list of conditions and the following disclaimer as
+ *    the first lines of this file unmodified other than the possible
+ *    addition of one or more copyright notices.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice(s), this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER(S) ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
+ * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/types.h>
+#include <sys/poll.h>
+#include "libc_private.h"
+
+__weak_reference(__sys_ppoll, __ppoll);
+
+#pragma weak ppoll
+int
+ppoll(struct pollfd pfd[], nfds_t nfds, const struct timespec *__restrict
+    timeout, const sigset_t *__restrict newsigmask)
+{
+
+	return (((int (*)(struct pollfd *, nfds_t, const struct timespec *,
+	    const sigset_t *)) __libc_interposing[INTERPOS_ppoll])(pfd, nfds,
+	    timeout, newsigmask));
+}
diff --git a/lib/libc/sys/wait6.c b/lib/libc/sys/wait6.c
new file mode 100644
index 0000000..f0e2999
--- /dev/null
+++ b/lib/libc/sys/wait6.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2014 The FreeBSD Foundation.
+ * All rights reserved.
+ *
+ * Portions of this software were developed by Konstantin Belousov
+ * under sponsorship from the FreeBSD Foundation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice(s), this list of conditions and the following disclaimer as
+ *    the first lines of this file unmodified other than the possible
+ *    addition of one or more copyright notices.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice(s), this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER(S) ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
+ * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include "libc_private.h"
+
+__weak_reference(__sys_wait6, __wait6);
+
+#pragma weak wait6
+pid_t
+wait6(idtype_t idtype, id_t id, int *status, int options, struct __wrusage *ru,
+    siginfo_t *infop)
+{
+
+	return (((pid_t (*)(idtype_t, id_t, int *, int, struct __wrusage *,
+	    siginfo_t *))__libc_interposing[INTERPOS_wait6])(idtype, id,
+	    status, options, ru, infop));
+}
diff --git a/lib/libthr/thread/thr_syscalls.c b/lib/libthr/thread/thr_syscalls.c
index e71bf4a..2869358 100644
--- a/lib/libthr/thread/thr_syscalls.c
+++ b/lib/libthr/thread/thr_syscalls.c
@@ -321,6 +321,21 @@ __thr_poll(struct pollfd *fds, unsigned int nfds, int timeout)
 	return (ret);
 }
 
+static int
+__thr_ppoll(struct pollfd pfd[], nfds_t nfds, const struct timespec *
+    timeout, const sigset_t *newsigmask)
+{
+	struct pthread *curthread;
+	int ret;
+
+	curthread = _get_curthread();
+	_thr_cancel_enter(curthread);
+	ret = __sys_ppoll(pfd, nfds, timeout, newsigmask);
+	_thr_cancel_leave(curthread, ret == -1);
+
+	return (ret);
+}
+
 /*
  * Cancellation behavior:
  *   Thread may be canceled at start, but if the system call returns something,
@@ -543,6 +558,20 @@ __thr_wait4(pid_t pid, int *status, int options, struct rusage *rusage)
 	return (ret);
 }
 
+static pid_t
+__thr_wait6(idtype_t idtype, id_t id, int *status, int options,
+    struct __wrusage *ru, siginfo_t *infop)
+{
+	struct pthread *curthread;
+	pid_t ret;
+
+	curthread = _get_curthread();
+	_thr_cancel_enter(curthread);
+	ret = __sys_wait6(idtype, id, status, options, ru, infop);
+	_thr_cancel_leave(curthread, ret <= 0);
+	return (ret);
+}
+
 /*
  * Cancellation behavior:
  *   Thread may be canceled at start, but if the thread wrote some data,
@@ -623,6 +652,8 @@ __thr_interpose_libc(void)
 	SLOT(spinlock);
 	SLOT(spinunlock);
 	SLOT(kevent);
+	SLOT(wait6);
+	SLOT(ppoll);
 #undef SLOT
 	*(__libc_interposing_slot(
 	    INTERPOS__pthread_mutex_init_calloc_cb)) =
diff --git a/share/man/man3/pthread_testcancel.3 b/share/man/man3/pthread_testcancel.3
index 0b54290..fc412de 100644
--- a/share/man/man3/pthread_testcancel.3
+++ b/share/man/man3/pthread_testcancel.3
@@ -1,5 +1,5 @@
 .\" $FreeBSD$
-.Dd March 29, 2015
+.Dd April 16, 2015
 .Dt PTHREAD_TESTCANCEL 3
 .Os
 .Sh NAME
@@ -132,6 +132,7 @@ argument  is non-zero.
 .It Fn openat
 .It Fn pause
 .It Fn poll
+.It Fn ppoll
 .It Fn pselect
 .It Fn pthread_cond_timedwait
 .It Fn pthread_cond_wait
@@ -159,6 +160,8 @@ argument  is non-zero.
 .It Fn wait
 .It Fn wait3
 .It Fn wait4
+.It Fn wait6
+.It Fn waitid
 .It Fn waitpid
 .It Fn write
 .It Fn writev



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150417075933.GH2390>