[v5,04/11] core: add kqueue support

Submitted by Leonid Bobrov via wayland-devel on Feb. 13, 2019, 11:39 a.m.

Details

Message ID 20190213113916.75159-4-mazocomp@disroot.org
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Leonid Bobrov via wayland-devel Feb. 13, 2019, 11:39 a.m.
Signed-off-by: Leonid Bobrov <mazocomp@disroot.org>
---
 Makefile.am                              |   3 +-
 configure.ac                             |  23 +-
 doc/doxygen/Makefile.am                  |   3 +-
 src/{event-loop.c => event-loop-epoll.c} |   6 +-
 src/event-loop-kqueue.c                  | 800 +++++++++++++++++++++++
 src/wayland-os.c                         |  47 +-
 src/wayland-os.h                         |   2 +-
 tests/os-wrappers-test.c                 |  55 +-
 8 files changed, 920 insertions(+), 19 deletions(-)
 rename src/{event-loop.c => event-loop-epoll.c} (99%)
 create mode 100644 src/event-loop-kqueue.c

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 489f581..6eefc73 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -76,7 +76,8 @@  libwayland_server_la_LDFLAGS = -version-info 1:0:1
 libwayland_server_la_SOURCES =			\
 	src/wayland-server.c			\
 	src/wayland-shm.c			\
-	src/event-loop.c
+	src/event-loop-epoll.c			\
+	src/event-loop-kqueue.c
 
 nodist_libwayland_server_la_SOURCES =		\
 	protocol/wayland-server-protocol.h	\
diff --git a/configure.ac b/configure.ac
index f53408a..cf8d82d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,6 +65,15 @@  AC_SUBST(GCC_CFLAGS)
 AC_CHECK_HEADERS([sys/prctl.h])
 AC_CHECK_FUNCS([accept4 mkostemp posix_fallocate prctl])
 
+AC_CHECK_HEADERS([sys/epoll.h])
+if test "x$ac_cv_header_sys_epoll_h" != "xyes"; then
+	AC_CHECK_HEADERS([sys/event.h])
+	if test "x$ac_cv_header_sys_event_h" != "xyes"; then
+		AC_MSG_ERROR([Can't find sys/epoll.h or sys/event.h. Please ensure either epoll or kqueue is available.])
+	fi
+fi
+
+
 # *BSD don't have libdl, but they have its functions
 SAVE_LIBS="$LIBS"
 LIBS=
@@ -116,12 +125,14 @@  AC_SUBST([ICONDIR])
 
 if test "x$enable_libraries" = "xyes"; then
 	PKG_CHECK_MODULES(FFI, [libffi])
-	AC_CHECK_DECL(SFD_CLOEXEC,[],
-		      [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile wayland libraries")],
-		      [[#include <sys/signalfd.h>]])
-	AC_CHECK_DECL(TFD_CLOEXEC,[],
-		      [AC_MSG_ERROR("TFD_CLOEXEC is needed to compile wayland libraries")],
-		      [[#include <sys/timerfd.h>]])
+	if test "x$ac_cv_header_sys_epoll_h" == "xyes"; then
+		AC_CHECK_DECL(SFD_CLOEXEC,[],
+			      [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile wayland libraries")],
+			      [[#include <sys/signalfd.h>]])
+		AC_CHECK_DECL(TFD_CLOEXEC,[],
+			      [AC_MSG_ERROR("TFD_CLOEXEC is needed to compile wayland libraries")],
+			      [[#include <sys/timerfd.h>]])
+	fi
 	AC_CHECK_DECL(CLOCK_MONOTONIC,[],
 		      [AC_MSG_ERROR("CLOCK_MONOTONIC is needed to compile wayland libraries")],
 		      [[#include <time.h>]])
diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
index f8b0b3a..60bed53 100644
--- a/doc/doxygen/Makefile.am
+++ b/doc/doxygen/Makefile.am
@@ -19,7 +19,8 @@  scanned_src_files_Client = 				\
 
 scanned_src_files_Server = 				\
 	$(scanned_src_files_shared)			\
-	$(top_srcdir)/src/event-loop.c		\
+	$(top_srcdir)/src/event-loop-epoll.c	\
+	$(top_srcdir)/src/event-loop-kqueue.c	\
 	$(top_srcdir)/src/wayland-server.c	\
 	$(top_srcdir)/src/wayland-server.h	\
 	$(top_srcdir)/src/wayland-server-core.h	\
diff --git a/src/event-loop.c b/src/event-loop-epoll.c
similarity index 99%
rename from src/event-loop.c
rename to src/event-loop-epoll.c
index eb2dce6..4784f64 100644
--- a/src/event-loop.c
+++ b/src/event-loop-epoll.c
@@ -23,6 +23,9 @@ 
  * SOFTWARE.
  */
 
+#include "config.h"
+
+#ifdef HAVE_SYS_EPOLL_H
 #include <stddef.h>
 #include <stdio.h>
 #include <errno.h>
@@ -523,7 +526,7 @@  wl_event_loop_create(void)
 	if (loop == NULL)
 		return NULL;
 
-	loop->epoll_fd = wl_os_epoll_create_cloexec();
+	loop->epoll_fd = wl_os_event_create_cloexec();
 	if (loop->epoll_fd < 0) {
 		free(loop);
 		return NULL;
@@ -702,3 +705,4 @@  wl_event_loop_get_destroy_listener(struct wl_event_loop *loop,
 {
 	return wl_signal_get(&loop->destroy_signal, notify);
 }
+#endif
diff --git a/src/event-loop-kqueue.c b/src/event-loop-kqueue.c
new file mode 100644
index 0000000..3149f53
--- /dev/null
+++ b/src/event-loop-kqueue.c
@@ -0,0 +1,800 @@ 
+/*
+ * Copyright © 2008 Kristian Høgsberg
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#ifdef HAVE_SYS_EVENT_H
+#include <stddef.h>
+#include <stdio.h>
+#include <err.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <sys/types.h>
+#include <sys/event.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include "wayland-util.h"
+#include "wayland-private.h"
+#include "wayland-server-core.h"
+#include "wayland-os.h"
+
+/** \cond INTERNAL */
+
+struct wl_event_loop {
+	int event_fd;
+	struct wl_list check_list;
+	struct wl_list idle_list;
+	struct wl_list destroy_list;
+
+	struct wl_signal destroy_signal;
+};
+
+struct wl_event_source_interface {
+	int (*dispatch)(struct wl_event_source *source,
+			struct kevent *ev);
+};
+
+struct wl_event_source {
+	struct wl_event_source_interface *interface;
+	struct wl_event_loop *loop;
+	struct wl_list link;
+	void *data;
+	int fd;
+};
+
+struct wl_event_source_fd {
+	struct wl_event_source base;
+	wl_event_loop_fd_func_t func;
+	int fd;
+};
+
+/** \endcond */
+
+static int
+wl_event_source_fd_dispatch(struct wl_event_source *source,
+			    struct kevent *ev)
+{
+	struct wl_event_source_fd *fd_source = (struct wl_event_source_fd *) source;
+	uint32_t mask;
+
+	mask = 0;
+	if (ev->filter == EVFILT_READ)
+		mask |= WL_EVENT_READABLE;
+	if (ev->filter == EVFILT_WRITE)
+		mask |= WL_EVENT_WRITABLE;
+	if (ev->flags & EV_EOF)
+		mask |= WL_EVENT_HANGUP;
+	if (ev->flags & EV_ERROR)
+		mask |= WL_EVENT_ERROR;
+
+	return fd_source->func(source->fd, mask, source->data);
+}
+
+struct wl_event_source_interface fd_source_interface = {
+	wl_event_source_fd_dispatch,
+};
+
+static struct wl_event_source *
+add_source(struct wl_event_loop *loop,
+	   struct wl_event_source *source, uint32_t mask, void *data)
+{
+	source->loop = loop;
+	source->data = data;
+	wl_list_init(&source->link);
+
+	return source;
+}
+
+/** Create a file descriptor event source
+ *
+ * \param loop The event loop that will process the new source.
+ * \param fd The file descriptor to watch.
+ * \param mask A bitwise-or of which events to watch for: \c WL_EVENT_READABLE,
+ * \c WL_EVENT_WRITABLE.
+ * \param func The file descriptor dispatch function.
+ * \param data User data.
+ * \return A new file descriptor event source.
+ *
+ * The given file descriptor is initially watched for the events given in
+ * \c mask. This can be changed as needed with wl_event_source_fd_update().
+ *
+ * If it is possible that program execution causes the file descriptor to be
+ * read while leaving the data in a buffer without actually processing it,
+ * it may be necessary to register the file descriptor source to be re-checked,
+ * see wl_event_source_check(). This will ensure that the dispatch function
+ * gets called even if the file descriptor is not readable or writable
+ * anymore. This is especially useful with IPC libraries that automatically
+ * buffer incoming data, possibly as a side-effect of other operations.
+ *
+ * \sa wl_event_loop_fd_func_t
+ * \memberof wl_event_source
+ */
+WL_EXPORT struct wl_event_source *
+wl_event_loop_add_fd(struct wl_event_loop *loop,
+		     int fd, uint32_t mask,
+		     wl_event_loop_fd_func_t func,
+		     void *data)
+{
+	struct wl_event_source_fd *source;
+
+	struct kevent events[2];
+	unsigned int num_events = 0;
+
+	source = malloc(sizeof *source);
+	if (source == NULL)
+		return NULL;
+
+	source->base.interface = &fd_source_interface;
+	source->base.fd = wl_os_dupfd_cloexec(fd, 0);
+	source->func = func;
+	source->fd = fd;
+	add_source(loop, &source->base, mask, data);
+
+	if (source->base.fd < 0) {
+		fprintf(stderr, "could not add source\n: %m");
+		free(source);
+		return NULL;
+	}
+
+	if (mask & WL_EVENT_READABLE) {
+		EV_SET(&events[num_events], source->base.fd, EVFILT_READ,
+		      EV_ADD | EV_ENABLE, 0, 0, &source->base);
+		num_events++;
+	}
+
+	if (mask & WL_EVENT_WRITABLE) {
+		EV_SET(&events[num_events], source->base.fd, EVFILT_WRITE,
+		      EV_ADD | EV_ENABLE, 0, 0, &source->base);
+		num_events++;
+	}
+
+	if (kevent(loop->event_fd, events, num_events, NULL, 0, NULL) < 0) {
+		fprintf(stderr, "error adding source %i (%p) to loop %p: %s\n",
+		       source->fd, source, loop, strerror(errno));
+		close(source->base.fd);
+		free(source);
+		return NULL;
+	}
+
+	return &source->base;
+}
+
+/** Update a file descriptor source's event mask
+ *
+ * \param source The file descriptor event source to update.
+ * \param mask The new mask, a bitwise-or of: \c WL_EVENT_READABLE,
+ * \c WL_EVENT_WRITABLE.
+ * \return 0 on success, -1 on failure.
+ *
+ * This changes which events, readable and/or writable, cause the dispatch
+ * callback to be called on.
+ *
+ * File descriptors are usually writable to begin with, so they do not need to
+ * be polled for writable until a write actually fails. When a write fails,
+ * the event mask can be changed to poll for readable and writable, delivering
+ * a dispatch callback when it is possible to write more. Once all data has
+ * been written, the mask can be changed to poll only for readable to avoid
+ * busy-looping on dispatch.
+ *
+ * \sa wl_event_loop_add_fd()
+ * \memberof wl_event_source
+ */
+WL_EXPORT int
+wl_event_source_fd_update(struct wl_event_source *source, uint32_t mask)
+{
+	struct wl_event_loop *loop = source->loop;
+	struct kevent events[2];
+	unsigned int num_events = 0;
+
+	if (mask & WL_EVENT_READABLE) {
+		EV_SET(&events[num_events], source->fd, EVFILT_READ,
+		       EV_ADD | EV_ENABLE, 0, 0, source);
+		num_events++;
+	}
+
+	if (mask & WL_EVENT_WRITABLE) {
+		EV_SET(&events[num_events], source->fd, EVFILT_WRITE,
+		       EV_ADD | EV_ENABLE, 0, 0, source);
+		num_events++;
+	}
+
+	return kevent(loop->event_fd, events, num_events, NULL, 0, NULL);
+}
+
+/** \cond INTERNAL */
+
+struct wl_event_source_timer {
+	struct wl_event_source base;
+	wl_event_loop_timer_func_t func;
+};
+
+/** \endcond */
+
+static int
+wl_event_source_timer_dispatch(struct wl_event_source *source,
+			       struct kevent *ev)
+{
+	struct wl_event_source_timer *timer_source =
+		(struct wl_event_source_timer *) source;
+	uint64_t expires;
+
+	expires = ev->data;    /* XXX unused?! */
+	return timer_source->func(timer_source->base.data);
+}
+
+struct wl_event_source_interface timer_source_interface = {
+	wl_event_source_timer_dispatch,
+};
+
+/** Create a timer event source
+ *
+ * \param loop The event loop that will process the new source.
+ * \param func The timer dispatch function.
+ * \param data User data.
+ * \return A new timer event source.
+ *
+ * The timer is initially disarmed. It needs to be armed with a call to
+ * wl_event_source_timer_update() before it can trigger a dispatch call.
+ *
+ * \sa wl_event_loop_timer_func_t
+ * \memberof wl_event_source
+ */
+WL_EXPORT struct wl_event_source *
+wl_event_loop_add_timer(struct wl_event_loop *loop,
+			wl_event_loop_timer_func_t func,
+			void *data)
+{
+	static int next_timer_id = 0;
+	struct wl_event_source_timer *source;
+
+	source = malloc(sizeof *source);
+	if (source == NULL)
+		return NULL;
+	source->base.interface = &timer_source_interface;
+	source->base.fd = next_timer_id++;
+	source->base.loop = loop;
+	source->base.data = data;
+	source->func = func;
+	wl_list_init(&source->base.link);
+
+	return &source->base;
+}
+
+/** Arm or disarm a timer
+ *
+ * \param source The timer event source to modify.
+ * \param ms_delay The timeout in milliseconds.
+ * \return 0 on success, -1 on failure.
+ *
+ * If the timeout is zero, the timer is disarmed.
+ *
+ * If the timeout is non-zero, the timer is set to expire after the given
+ * timeout in milliseconds. When the timer expires, the dispatch function
+ * set with wl_event_loop_add_timer() is called once from
+ * wl_event_loop_dispatch(). If another dispatch is desired after another
+ * expiry, wl_event_source_timer_update() needs to be called again.
+ *
+ * \memberof wl_event_source
+ */
+WL_EXPORT int
+wl_event_source_timer_update(struct wl_event_source *source, int ms_delay)
+{
+	struct kevent ev;
+
+	if (ms_delay == 0) {
+		EV_SET(&ev, source->fd, EVFILT_TIMER, EV_DISABLE, 0,
+		       ms_delay, source);
+	} else {
+		EV_SET(&ev, source->fd, EVFILT_TIMER,
+		       EV_ADD | EV_ENABLE | EV_DISPATCH, 0, ms_delay, source);
+	}
+
+	if (kevent(source->loop->event_fd, &ev, 1, NULL, 0, NULL) < 0) {
+		fprintf(stderr, "could not set kqueue timer: %s",
+		        strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+/** \cond INTERNAL */
+
+struct wl_event_source_signal {
+	struct wl_event_source base;
+	int signal_number;
+	wl_event_loop_signal_func_t func;
+};
+
+/** \endcond */
+
+static int
+wl_event_source_signal_dispatch(struct wl_event_source *source,
+				struct kevent *ev)
+{
+	struct wl_event_source_signal *signal_source;
+
+	signal_source = (struct wl_event_source_signal *) source;
+
+	return signal_source->func(signal_source->base.fd,
+	                           signal_source->base.data);
+}
+
+struct wl_event_source_interface signal_source_interface = {
+	wl_event_source_signal_dispatch,
+};
+
+/** Create a POSIX signal event source
+ *
+ * \param loop The event loop that will process the new source.
+ * \param signal_number Number of the signal to watch for.
+ * \param func The signal dispatch function.
+ * \param data User data.
+ * \return A new signal event source.
+ *
+ * This function blocks the normal delivery of the given signal in the calling
+ * thread, and creates a "watch" for it. Signal delivery no longer happens
+ * asynchronously, but by wl_event_loop_dispatch() calling the dispatch
+ * callback function \c func.
+ *
+ * It is the caller's responsibility to ensure that all other threads have
+ * also blocked the signal.
+ *
+ * \sa wl_event_loop_signal_func_t
+ * \memberof wl_event_source
+ */
+WL_EXPORT struct wl_event_source *
+wl_event_loop_add_signal(struct wl_event_loop *loop,
+			int signal_number,
+			wl_event_loop_signal_func_t func,
+			void *data)
+{
+	struct wl_event_source_signal *source;
+	sigset_t mask;
+	struct kevent ev;
+
+	source = malloc(sizeof *source);
+	if (source == NULL)
+		return NULL;
+
+	source->base.interface = &signal_source_interface;
+	source->signal_number = signal_number;
+	source->func = func;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, signal_number);
+	sigprocmask(SIG_BLOCK, &mask, NULL);
+
+	source->base.fd = 0;
+	add_source(loop, &source->base, WL_EVENT_READABLE, data);
+
+	EV_SET(&ev, signal_number, EVFILT_SIGNAL, EV_ADD | EV_ENABLE, 0, 0,
+	       source);
+
+	if (kevent(loop->event_fd, &ev, 1, NULL, 0, NULL) < 0) {
+		fprintf(stderr, "error adding signal for %i (%p), %p: %s\n",
+			signal_number, source, loop, strerror(errno));
+		free(source);
+		return NULL;
+	}
+
+	return &source->base;
+}
+
+/** \cond INTERNAL */
+
+struct wl_event_source_idle {
+	struct wl_event_source base;
+	wl_event_loop_idle_func_t func;
+};
+
+/** \endcond */
+
+struct wl_event_source_interface idle_source_interface = {
+	NULL,
+};
+
+/** Create an idle task
+ *
+ * \param loop The event loop that will process the new task.
+ * \param func The idle task dispatch function.
+ * \param data User data.
+ * \return A new idle task (an event source).
+ *
+ * Idle tasks are dispatched before wl_event_loop_dispatch() goes to sleep.
+ * See wl_event_loop_dispatch() for more details.
+ *
+ * Idle tasks fire once, and are automatically destroyed right after the
+ * callback function has been called.
+ *
+ * An idle task can be cancelled before the callback has been called by
+ * wl_event_source_remove(). Calling wl_event_source_remove() after or from
+ * within the callback results in undefined behaviour.
+ *
+ * \sa wl_event_loop_idle_func_t
+ * \memberof wl_event_source
+ */
+WL_EXPORT struct wl_event_source *
+wl_event_loop_add_idle(struct wl_event_loop *loop,
+		       wl_event_loop_idle_func_t func,
+		       void *data)
+{
+	struct wl_event_source_idle *source;
+
+	source = malloc(sizeof *source);
+	if (source == NULL)
+		return NULL;
+
+	source->base.interface = &idle_source_interface;
+	source->base.loop = loop;
+	source->base.fd = -1;
+
+	source->func = func;
+	source->base.data = data;
+
+	wl_list_insert(loop->idle_list.prev, &source->base.link);
+
+	return &source->base;
+}
+
+/** Mark event source to be re-checked
+ *
+ * \param source The event source to be re-checked.
+ *
+ * This function permanently marks the event source to be re-checked after
+ * the normal dispatch of sources in wl_event_loop_dispatch(). Re-checking
+ * will keep iterating over all such event sources until the dispatch
+ * function for them all returns zero.
+ *
+ * Re-checking is used on sources that may become ready to dispatch as a
+ * side-effect of dispatching themselves or other event sources, including idle
+ * sources. Re-checking ensures all the incoming events have been fully drained
+ * before wl_event_loop_dispatch() returns.
+ *
+ * \memberof wl_event_source
+ */
+WL_EXPORT void
+wl_event_source_check(struct wl_event_source *source)
+{
+	wl_list_insert(source->loop->check_list.prev, &source->link);
+}
+
+/** Remove an event source from its event loop
+ *
+ * \param source The event source to be removed.
+ * \return Zero.
+ *
+ * The event source is removed from the event loop it was created for,
+ * and is effectively destroyed. This invalidates \c source .
+ * The dispatch function of the source will no longer be called through this
+ * source.
+ *
+ * \memberof wl_event_source
+ */
+WL_EXPORT int
+wl_event_source_remove(struct wl_event_source *source)
+{
+	struct wl_event_loop *loop = source->loop;
+	int ret = 0, saved_errno = 0;
+
+	/*
+	 * Since BSD doesn't treat all event sources as FDs, we need to
+	 * differentiate by source interface.
+	 */
+	if (source->interface == &fd_source_interface && source->fd >= 0) {
+		struct kevent ev[2];
+		int _ret[2], _saved_errno[2];
+
+		/*
+		 * We haven't stored state about the mask used when adding the
+		 * source, so we have to try and remove both READ and WRITE
+		 * filters. One may fail, which is OK. Removal of the source has
+		 * only failed if _both_ kevent() calls fail. We have to do two
+		 * kevent() calls so that we can get independent return values
+		 * for the two kevents.
+		 */
+		EV_SET(&ev[0], source->fd, EVFILT_READ, EV_DELETE, 0, 0,
+		      source);
+		EV_SET(&ev[1], source->fd, EVFILT_WRITE, EV_DELETE, 0, 0,
+		      source);
+
+		_ret[0] = kevent(loop->event_fd, &ev[0], 1, NULL, 0, NULL);
+		_saved_errno[0] = errno;
+		_ret[1] = kevent(loop->event_fd, &ev[1], 1, NULL, 0, NULL);
+		_saved_errno[1] = errno;
+
+		if (_ret[0] >= _ret[1]) {
+			ret = _ret[0];
+			saved_errno = _saved_errno[0];
+		} else {
+			ret = _ret[1];
+			saved_errno = _saved_errno[1];
+		}
+
+		close(source->fd);
+	} else if (source->interface == &timer_source_interface) {
+		struct kevent ev;
+
+		EV_SET(&ev, source->fd, EVFILT_TIMER, EV_DELETE, 0, 0, source);
+		ret = kevent(loop->event_fd, &ev, 1, NULL, 0, NULL);
+		saved_errno = errno;
+	} else if (source->interface == &signal_source_interface) {
+		struct kevent ev;
+		int signal_number;
+		struct wl_event_source_signal *_source;
+
+		/* Only one kevent() call needed. */
+		_source = (struct wl_event_source_signal *) source;
+		signal_number = _source->signal_number;
+
+		EV_SET(&ev, signal_number, EVFILT_SIGNAL, EV_DELETE, 0, 0,
+		      source);
+		ret = kevent(loop->event_fd, &ev, 1, NULL, 0, NULL);
+		saved_errno = errno;
+	}
+
+	/* Handle any errors from kevent() calls. */
+	if (ret < 0) {
+		fprintf(stderr,
+		        "error removing event (%i) from kqueue: %s\n",
+		        source->fd, strerror(saved_errno));
+	}
+
+	/* Tidy up the source. */
+	source->fd = -1;
+
+	wl_list_remove(&source->link);
+	wl_list_insert(&loop->destroy_list, &source->link);
+
+	return 0;
+}
+
+static void
+wl_event_loop_process_destroy_list(struct wl_event_loop *loop)
+{
+	struct wl_event_source *source, *next;
+
+	wl_list_for_each_safe(source, next, &loop->destroy_list, link)
+		free(source);
+
+	wl_list_init(&loop->destroy_list);
+}
+
+/** Create a new event loop context
+ *
+ * \return A new event loop context object.
+ *
+ * This creates a new event loop context. Initially this context is empty.
+ * Event sources need to be explicitly added to it.
+ *
+ * Normally the event loop is run by calling wl_event_loop_dispatch() in
+ * a loop until the program terminates. Alternatively, an event loop can be
+ * embedded in another event loop by its file descriptor, see
+ * wl_event_loop_get_fd().
+ *
+ * \memberof wl_event_loop
+ */
+WL_EXPORT struct wl_event_loop *
+wl_event_loop_create(void)
+{
+	struct wl_event_loop *loop;
+
+	loop = malloc(sizeof *loop);
+	if (loop == NULL)
+		return NULL;
+
+	loop->event_fd = wl_os_event_create_cloexec();
+	if (loop->event_fd < 0) {
+		free(loop);
+		return NULL;
+	}
+	wl_list_init(&loop->check_list);
+	wl_list_init(&loop->idle_list);
+	wl_list_init(&loop->destroy_list);
+
+	wl_signal_init(&loop->destroy_signal);
+
+	return loop;
+}
+
+/** Destroy an event loop context
+ *
+ * \param loop The event loop to be destroyed.
+ *
+ * This emits the event loop destroy signal, closes the event loop file
+ * descriptor, and frees \c loop.
+ *
+ * If the event loop has existing sources, those cannot be safely removed
+ * afterwards. Therefore one must call wl_event_source_remove() on all
+ * event sources before destroying the event loop context.
+ *
+ * \memberof wl_event_loop
+ */
+WL_EXPORT void
+wl_event_loop_destroy(struct wl_event_loop *loop)
+{
+	wl_signal_emit(&loop->destroy_signal, loop);
+
+	wl_event_loop_process_destroy_list(loop);
+	close(loop->event_fd);
+	free(loop);
+}
+
+static bool
+post_dispatch_check(struct wl_event_loop *loop)
+{
+	struct kevent ev;
+	struct wl_event_source *source, *next;
+	bool needs_recheck = false;
+
+	wl_list_for_each_safe(source, next, &loop->check_list, link) {
+		int dispatch_result;
+
+		dispatch_result = source->interface->dispatch(source, &ev);
+		if (dispatch_result < 0) {
+			wl_log("Source dispatch function returned negative value!");
+			wl_log("This would previously accidentally suppress a follow-up dispatch");
+		}
+		needs_recheck |= dispatch_result != 0;
+	}
+
+	return needs_recheck;
+}
+
+/** Dispatch the idle sources
+ *
+ * \param loop The event loop whose idle sources are dispatched.
+ *
+ * \sa wl_event_loop_add_idle()
+ * \memberof wl_event_loop
+ */
+WL_EXPORT void
+wl_event_loop_dispatch_idle(struct wl_event_loop *loop)
+{
+	struct wl_event_source_idle *source;
+
+	while (!wl_list_empty(&loop->idle_list)) {
+		source = container_of(loop->idle_list.next,
+				      struct wl_event_source_idle, base.link);
+		source->func(source->base.data);
+		wl_event_source_remove(&source->base);
+	}
+}
+
+/** Wait for events and dispatch them
+ *
+ * \param loop The event loop whose sources to wait for.
+ * \param timeout The polling timeout in milliseconds.
+ * \return 0 for success, -1 for polling error.
+ *
+ * All the associated event sources are polled. This function blocks until
+ * any event source delivers an event (idle sources excluded), or the timeout
+ * expires. A timeout of -1 disables the timeout, causing the function to block
+ * indefinitely. A timeout of zero causes the poll to always return immediately.
+ *
+ * All idle sources are dispatched before blocking. An idle source is destroyed
+ * when it is dispatched. After blocking, all other ready sources are
+ * dispatched. Then, idle sources are dispatched again, in case the dispatched
+ * events created idle sources. Finally, all sources marked with
+ * wl_event_source_check() are dispatched in a loop until their dispatch
+ * functions all return zero.
+ *
+ * \memberof wl_event_loop
+ */
+WL_EXPORT int
+wl_event_loop_dispatch(struct wl_event_loop *loop, int timeout)
+{
+	struct kevent ev[64];
+	struct wl_event_source *source;
+	int i, count;
+	struct timespec timeout_spec;
+
+	wl_event_loop_dispatch_idle(loop);
+
+	/* timeout is provided in milliseconds */
+	timeout_spec.tv_sec = timeout / 1000;
+	timeout_spec.tv_nsec = (timeout % 1000) * 1000000;
+
+	count = kevent(loop->event_fd, NULL, 0, ev, ARRAY_LENGTH(ev),
+	               (timeout != -1) ? &timeout_spec : NULL);
+	if (count < 0)
+		return -1;
+
+	for (i = 0; i < count; i++) {
+		source = ev[i].udata;
+		if (source->fd != -1) {
+		       source->interface->dispatch(source, &ev[i]);
+		}
+	}
+
+	wl_event_loop_process_destroy_list(loop);
+
+	wl_event_loop_dispatch_idle(loop);
+
+	while (post_dispatch_check(loop));
+
+	return 0;
+}
+
+/** Get the event loop file descriptor
+ *
+ * \param loop The event loop context.
+ * \return The aggregate file descriptor.
+ *
+ * This function returns the aggregate file descriptor, that represents all
+ * the event sources (idle sources excluded) associated with the given event
+ * loop context. When any event source makes an event available, it will be
+ * reflected in the aggregate file descriptor.
+ *
+ * When the aggregate file descriptor delivers an event, one can call
+ * wl_event_loop_dispatch() on the event loop context to dispatch all the
+ * available events.
+ *
+ * \memberof wl_event_loop
+ */
+WL_EXPORT int
+wl_event_loop_get_fd(struct wl_event_loop *loop)
+{
+	return loop->event_fd;
+}
+
+/** Register a destroy listener for an event loop context
+ *
+ * \param loop The event loop context whose destruction to listen for.
+ * \param listener The listener with the callback to be called.
+ *
+ * \sa wl_listener
+ * \memberof wl_event_loop
+ */
+WL_EXPORT void
+wl_event_loop_add_destroy_listener(struct wl_event_loop *loop,
+				   struct wl_listener *listener)
+{
+	wl_signal_add(&loop->destroy_signal, listener);
+}
+
+/** Get the listener struct for the specified callback
+ *
+ * \param loop The event loop context to inspect.
+ * \param notify The destroy callback to find.
+ * \return The wl_listener registered to the event loop context with
+ * the given callback pointer.
+ *
+ * \memberof wl_event_loop
+ */
+WL_EXPORT struct wl_listener *
+wl_event_loop_get_destroy_listener(struct wl_event_loop *loop,
+				   wl_notify_func_t notify)
+{
+	return wl_signal_get(&loop->destroy_signal, notify);
+}
+#endif
diff --git a/src/wayland-os.c b/src/wayland-os.c
index 93b6f5f..7c8a993 100644
--- a/src/wayland-os.c
+++ b/src/wayland-os.c
@@ -23,6 +23,8 @@ 
  * SOFTWARE.
  */
 
+#include "config.h"
+
 #define _GNU_SOURCE
 
 #include <sys/types.h>
@@ -30,9 +32,13 @@ 
 #include <unistd.h>
 #include <fcntl.h>
 #include <errno.h>
+#ifdef HAVE_SYS_EPOLL_H
 #include <sys/epoll.h>
+#endif
+#ifdef HAVE_SYS_EVENT_H
+#include <sys/event.h>
+#endif
 
-#include "../config.h"
 #include "wayland-os.h"
 
 static int
@@ -62,26 +68,50 @@  wl_os_socket_cloexec(int domain, int type, int protocol)
 {
 	int fd;
 
+#ifdef SOCK_CLOEXEC
 	fd = socket(domain, type | SOCK_CLOEXEC, protocol);
 	if (fd >= 0)
 		return fd;
 	if (errno != EINVAL)
 		return -1;
+#endif
 
 	fd = socket(domain, type, protocol);
 	return set_cloexec_or_close(fd);
 }
 
+int
+wl_os_socketpair_cloexec(int domain, int type, int protocol, int sv[2])
+{
+       int retval;
+
+#ifdef SOCK_CLOEXEC
+       retval = socketpair(domain, type | SOCK_CLOEXEC, protocol, sv);
+       if (retval >= 0)
+               return retval;
+       if (errno != EINVAL)
+               return -1;
+#endif
+
+       retval = socketpair(domain, type, protocol, sv);
+       if (set_cloexec_or_close(sv[0]) < 0 || set_cloexec_or_close(sv[1]) < 0)
+               retval = -1;
+
+       return retval;
+}
+
 int
 wl_os_dupfd_cloexec(int fd, long minfd)
 {
 	int newfd;
 
+#ifdef F_DUPFD_CLOEXEC
 	newfd = fcntl(fd, F_DUPFD_CLOEXEC, minfd);
 	if (newfd >= 0)
 		return newfd;
 	if (errno != EINVAL)
 		return -1;
+#endif
 
 	newfd = fcntl(fd, F_DUPFD, minfd);
 	return set_cloexec_or_close(newfd);
@@ -123,17 +153,20 @@  wl_os_recvmsg_cloexec(int sockfd, struct msghdr *msg, int flags)
 {
 	ssize_t len;
 
+#ifdef MSG_CMSG_CLOEXEC
 	len = recvmsg(sockfd, msg, flags | MSG_CMSG_CLOEXEC);
 	if (len >= 0)
 		return len;
 	if (errno != EINVAL)
 		return -1;
+#endif
 
 	return recvmsg_cloexec_fallback(sockfd, msg, flags);
 }
 
+#if defined(HAVE_SYS_EPOLL_H)
 int
-wl_os_epoll_create_cloexec(void)
+wl_os_event_create_cloexec(void)
 {
 	int fd;
 
@@ -148,6 +181,16 @@  wl_os_epoll_create_cloexec(void)
 	fd = epoll_create(1);
 	return set_cloexec_or_close(fd);
 }
+#elif defined(HAVE_SYS_EVENT_H)
+int
+wl_os_event_create_cloexec(void)
+{
+	int fd;
+
+	fd = kqueue();
+	return set_cloexec_or_close(fd);
+}
+#endif
 
 int
 wl_os_accept_cloexec(int sockfd, struct sockaddr *addr, socklen_t *addrlen)
diff --git a/src/wayland-os.h b/src/wayland-os.h
index f51efaa..86685e0 100644
--- a/src/wayland-os.h
+++ b/src/wayland-os.h
@@ -36,7 +36,7 @@  ssize_t
 wl_os_recvmsg_cloexec(int sockfd, struct msghdr *msg, int flags);
 
 int
-wl_os_epoll_create_cloexec(void);
+wl_os_event_create_cloexec(void);
 
 int
 wl_os_accept_cloexec(int sockfd, struct sockaddr *addr, socklen_t *addrlen);
diff --git a/tests/os-wrappers-test.c b/tests/os-wrappers-test.c
index 102622c..e7bcd6c 100644
--- a/tests/os-wrappers-test.c
+++ b/tests/os-wrappers-test.c
@@ -26,6 +26,7 @@ 
 
 #define _GNU_SOURCE
 
+#include <err.h>
 #include <stdlib.h>
 #include <stdint.h>
 #include <assert.h>
@@ -38,7 +39,11 @@ 
 #include <stdarg.h>
 #include <fcntl.h>
 #include <stdio.h>
-#include <sys/epoll.h>
+#ifdef HAVE_SYS_EPOLL_H
+# include <sys/epoll.h>
+#else
+# include <sys/event.h>
+#endif
 
 #include "wayland-private.h"
 #include "test-runner.h"
@@ -55,8 +60,15 @@  static int wrapped_calls_fcntl;
 static ssize_t (*real_recvmsg)(int, struct msghdr *, int);
 static int wrapped_calls_recvmsg;
 
+#ifdef HAVE_SYS_EPOLL_H
 static int (*real_epoll_create1)(int);
 static int wrapped_calls_epoll_create1;
+#endif
+
+#if HAVE_SYS_EVENT_H
+static int (*real_kqueue)(void);
+static int wrapped_calls_kqueue;
+#endif
 
 static void
 init_fallbacks(int do_fallbacks)
@@ -65,7 +77,12 @@  init_fallbacks(int do_fallbacks)
 	real_socket = dlsym(RTLD_NEXT, "socket");
 	real_fcntl = dlsym(RTLD_NEXT, "fcntl");
 	real_recvmsg = dlsym(RTLD_NEXT, "recvmsg");
+#ifdef HAVE_SYS_EPOLL_H
 	real_epoll_create1 = dlsym(RTLD_NEXT, "epoll_create1");
+#endif
+#ifdef HAVE_SYS_EVENT_H
+	real_kqueue = dlsym(RTLD_NEXT, "kqueue");
+#endif
 }
 
 __attribute__ ((visibility("default"))) int
@@ -81,6 +98,8 @@  socket(int domain, int type, int protocol)
 	return real_socket(domain, type, protocol);
 }
 
+/* won't work on OpenBSD, since real fcntl have to be called early */
+#if !defined(__OpenBSD__)
 __attribute__ ((visibility("default"))) int
 fcntl(int fd, int cmd, ...)
 {
@@ -100,6 +119,7 @@  fcntl(int fd, int cmd, ...)
 
 	return real_fcntl(fd, cmd, arg);
 }
+#endif
 
 __attribute__ ((visibility("default"))) ssize_t
 recvmsg(int sockfd, struct msghdr *msg, int flags)
@@ -114,6 +134,7 @@  recvmsg(int sockfd, struct msghdr *msg, int flags)
 	return real_recvmsg(sockfd, msg, flags);
 }
 
+#ifdef HAVE_SYS_EPOLL_H
 __attribute__ ((visibility("default"))) int
 epoll_create1(int flags)
 {
@@ -127,6 +148,23 @@  epoll_create1(int flags)
 
 	return real_epoll_create1(flags);
 }
+#endif
+
+#ifdef HAVE_SYS_EVENT_H
+__attribute__ ((visibility("default"))) int
+kqueue(void)
+{
+	wrapped_calls_kqueue++;
+
+	if (fall_back) {
+		wrapped_calls_kqueue++; /* kqueue() not wrapped */
+		errno = EINVAL;
+		return -1;
+	}
+
+	return real_kqueue();
+}
+#endif
 
 static void
 do_os_wrappers_socket_cloexec(int n)
@@ -184,6 +222,7 @@  do_os_wrappers_dupfd_cloexec(int n)
 	 * Must have 4 calls if falling back, but must also allow
 	 * falling back without a forced fallback.
 	 */
+	warnx("wrapped_calls_fcntl is %d, should be larger than %d", wrapped_calls_fcntl, n);
 	assert(wrapped_calls_fcntl > n);
 
 	exec_fd_leak_check(nr_fds);
@@ -335,14 +374,14 @@  TEST(os_wrappers_recvmsg_cloexec_fallback)
 }
 
 static void
-do_os_wrappers_epoll_create_cloexec(int n)
+do_os_wrappers_event_create_cloexec(int n)
 {
 	int fd;
 	int nr_fds;
 
 	nr_fds = count_open_fds();
 
-	fd = wl_os_epoll_create_cloexec();
+	fd = wl_os_event_create_cloexec();
 	assert(fd >= 0);
 
 #ifdef EPOLL_CLOEXEC
@@ -354,16 +393,18 @@  do_os_wrappers_epoll_create_cloexec(int n)
 	exec_fd_leak_check(nr_fds);
 }
 
-TEST(os_wrappers_epoll_create_cloexec)
+TEST(os_wrappers_event_create_cloexec)
 {
 	init_fallbacks(0);
-	do_os_wrappers_epoll_create_cloexec(1);
+	do_os_wrappers_event_create_cloexec(1);
 }
 
-TEST(os_wrappers_epoll_create_cloexec_fallback)
+TEST(os_wrappers_event_create_cloexec_fallback)
 {
 	init_fallbacks(1);
-	do_os_wrappers_epoll_create_cloexec(2);
+	do_os_wrappers_event_create_cloexec(2);
 }
 
 /* FIXME: add tests for wl_os_accept_cloexec() */
+
+/* FIXME: add tests for kqueue() */

Comments

On Wed, 13 Feb 2019 13:39:09 +0200
Leonid Bobrov via wayland-devel <wayland-devel@lists.freedesktop.org> wrote:

> Signed-off-by: Leonid Bobrov <mazocomp@disroot.org>
> ---
>  Makefile.am                              |   3 +-
>  configure.ac                             |  23 +-
>  doc/doxygen/Makefile.am                  |   3 +-
>  src/{event-loop.c => event-loop-epoll.c} |   6 +-
>  src/event-loop-kqueue.c                  | 800 +++++++++++++++++++++++
>  src/wayland-os.c                         |  47 +-
>  src/wayland-os.h                         |   2 +-
>  tests/os-wrappers-test.c                 |  55 +-
>  8 files changed, 920 insertions(+), 19 deletions(-)
>  rename src/{event-loop.c => event-loop-epoll.c} (99%)
>  create mode 100644 src/event-loop-kqueue.c
> 

Hi Leonid,

I think the architectural approach here is good, there are some details
below that I have comments on, and I cannot review the kqueue
implementation at all, so someone else needs to give their Reviewed-by
on that (or if the original author was someone else, then your
acceptance of it could count as that review).

This patch is obviously one that I'd like to see the CI work running at
least in a branch somewhere first to make sure that CI will be
integrated as well.

> diff --git a/Makefile.am b/Makefile.am
> index 489f581..6eefc73 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -76,7 +76,8 @@ libwayland_server_la_LDFLAGS = -version-info 1:0:1
>  libwayland_server_la_SOURCES =			\
>  	src/wayland-server.c			\
>  	src/wayland-shm.c			\
> -	src/event-loop.c
> +	src/event-loop-epoll.c			\
> +	src/event-loop-kqueue.c
>  
>  nodist_libwayland_server_la_SOURCES =		\
>  	protocol/wayland-server-protocol.h	\
> diff --git a/configure.ac b/configure.ac
> index f53408a..cf8d82d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -65,6 +65,15 @@ AC_SUBST(GCC_CFLAGS)
>  AC_CHECK_HEADERS([sys/prctl.h])
>  AC_CHECK_FUNCS([accept4 mkostemp posix_fallocate prctl])
>  
> +AC_CHECK_HEADERS([sys/epoll.h])
> +if test "x$ac_cv_header_sys_epoll_h" != "xyes"; then
> +	AC_CHECK_HEADERS([sys/event.h])
> +	if test "x$ac_cv_header_sys_event_h" != "xyes"; then
> +		AC_MSG_ERROR([Can't find sys/epoll.h or sys/event.h. Please ensure either epoll or kqueue is available.])
> +	fi
> +fi
> +
> +
>  # *BSD don't have libdl, but they have its functions
>  SAVE_LIBS="$LIBS"
>  LIBS=
> @@ -116,12 +125,14 @@ AC_SUBST([ICONDIR])
>  
>  if test "x$enable_libraries" = "xyes"; then
>  	PKG_CHECK_MODULES(FFI, [libffi])
> -	AC_CHECK_DECL(SFD_CLOEXEC,[],
> -		      [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile wayland libraries")],
> -		      [[#include <sys/signalfd.h>]])
> -	AC_CHECK_DECL(TFD_CLOEXEC,[],
> -		      [AC_MSG_ERROR("TFD_CLOEXEC is needed to compile wayland libraries")],
> -		      [[#include <sys/timerfd.h>]])
> +	if test "x$ac_cv_header_sys_epoll_h" == "xyes"; then
> +		AC_CHECK_DECL(SFD_CLOEXEC,[],
> +			      [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile wayland libraries")],
> +			      [[#include <sys/signalfd.h>]])
> +		AC_CHECK_DECL(TFD_CLOEXEC,[],
> +			      [AC_MSG_ERROR("TFD_CLOEXEC is needed to compile wayland libraries")],
> +			      [[#include <sys/timerfd.h>]])
> +	fi

Ok, this seems fine.

>  	AC_CHECK_DECL(CLOCK_MONOTONIC,[],
>  		      [AC_MSG_ERROR("CLOCK_MONOTONIC is needed to compile wayland libraries")],
>  		      [[#include <time.h>]])
> diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
> index f8b0b3a..60bed53 100644
> --- a/doc/doxygen/Makefile.am
> +++ b/doc/doxygen/Makefile.am
> @@ -19,7 +19,8 @@ scanned_src_files_Client = 				\
>  
>  scanned_src_files_Server = 				\
>  	$(scanned_src_files_shared)			\
> -	$(top_srcdir)/src/event-loop.c		\
> +	$(top_srcdir)/src/event-loop-epoll.c	\
> +	$(top_srcdir)/src/event-loop-kqueue.c	\
>  	$(top_srcdir)/src/wayland-server.c	\
>  	$(top_srcdir)/src/wayland-server.h	\
>  	$(top_srcdir)/src/wayland-server-core.h	\
> diff --git a/src/event-loop.c b/src/event-loop-epoll.c
> similarity index 99%
> rename from src/event-loop.c
> rename to src/event-loop-epoll.c
> index eb2dce6..4784f64 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop-epoll.c
> @@ -23,6 +23,9 @@
>   * SOFTWARE.
>   */
>  
> +#include "config.h"
> +
> +#ifdef HAVE_SYS_EPOLL_H

Since we have the two .c files for the two different implementations,
rather than building always both with #ifdefs, you could gate in
Makefile.am which one of the two files gets built.

>  #include <stddef.h>
>  #include <stdio.h>
>  #include <errno.h>
> @@ -523,7 +526,7 @@ wl_event_loop_create(void)
>  	if (loop == NULL)
>  		return NULL;
>  
> -	loop->epoll_fd = wl_os_epoll_create_cloexec();
> +	loop->epoll_fd = wl_os_event_create_cloexec();
>  	if (loop->epoll_fd < 0) {
>  		free(loop);
>  		return NULL;
> @@ -702,3 +705,4 @@ wl_event_loop_get_destroy_listener(struct wl_event_loop *loop,
>  {
>  	return wl_signal_get(&loop->destroy_signal, notify);
>  }
> +#endif
> diff --git a/src/event-loop-kqueue.c b/src/event-loop-kqueue.c
> new file mode 100644
> index 0000000..3149f53
> --- /dev/null
> +++ b/src/event-loop-kqueue.c
> @@ -0,0 +1,800 @@
> +/*
> + * Copyright © 2008 Kristian Høgsberg

There is probably another copyright holder to add, isn't there?
The person who originally wrote the kqueue-based event loop implementation?

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:

...

> +/** Create an idle task
> + *
> + * \param loop The event loop that will process the new task.
> + * \param func The idle task dispatch function.
> + * \param data User data.
> + * \return A new idle task (an event source).
> + *
> + * Idle tasks are dispatched before wl_event_loop_dispatch() goes to sleep.
> + * See wl_event_loop_dispatch() for more details.
> + *
> + * Idle tasks fire once, and are automatically destroyed right after the
> + * callback function has been called.
> + *
> + * An idle task can be cancelled before the callback has been called by
> + * wl_event_source_remove(). Calling wl_event_source_remove() after or from
> + * within the callback results in undefined behaviour.
> + *
> + * \sa wl_event_loop_idle_func_t
> + * \memberof wl_event_source
> + */
> +WL_EXPORT struct wl_event_source *
> +wl_event_loop_add_idle(struct wl_event_loop *loop,
> +		       wl_event_loop_idle_func_t func,
> +		       void *data)
> +{
> +	struct wl_event_source_idle *source;
> +
> +	source = malloc(sizeof *source);
> +	if (source == NULL)
> +		return NULL;
> +
> +	source->base.interface = &idle_source_interface;
> +	source->base.loop = loop;
> +	source->base.fd = -1;
> +
> +	source->func = func;
> +	source->base.data = data;
> +
> +	wl_list_insert(loop->idle_list.prev, &source->base.link);
> +
> +	return &source->base;
> +}

It's a bit unfortunate to copy all these functions that must be
identical between the epoll and kqueue implementations. We could have a
event-loop-common.c for sharing these and a event-loop-private.h to go
with it. If you want to do that, then that would be a separate patch
before this one.


...

> diff --git a/src/wayland-os.c b/src/wayland-os.c
> index 93b6f5f..7c8a993 100644
> --- a/src/wayland-os.c
> +++ b/src/wayland-os.c
> @@ -23,6 +23,8 @@
>   * SOFTWARE.
>   */
>  
> +#include "config.h"
> +
>  #define _GNU_SOURCE
>  
>  #include <sys/types.h>
> @@ -30,9 +32,13 @@
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <errno.h>
> +#ifdef HAVE_SYS_EPOLL_H
>  #include <sys/epoll.h>
> +#endif
> +#ifdef HAVE_SYS_EVENT_H
> +#include <sys/event.h>
> +#endif
>  
> -#include "../config.h"
>  #include "wayland-os.h"
>  
>  static int
> @@ -62,26 +68,50 @@ wl_os_socket_cloexec(int domain, int type, int protocol)
>  {
>  	int fd;
>  
> +#ifdef SOCK_CLOEXEC
>  	fd = socket(domain, type | SOCK_CLOEXEC, protocol);
>  	if (fd >= 0)
>  		return fd;
>  	if (errno != EINVAL)
>  		return -1;
> +#endif
>  
>  	fd = socket(domain, type, protocol);
>  	return set_cloexec_or_close(fd);
>  }
>  
> +int
> +wl_os_socketpair_cloexec(int domain, int type, int protocol, int sv[2])
> +{
> +       int retval;
> +
> +#ifdef SOCK_CLOEXEC
> +       retval = socketpair(domain, type | SOCK_CLOEXEC, protocol, sv);
> +       if (retval >= 0)
> +               return retval;
> +       if (errno != EINVAL)
> +               return -1;
> +#endif
> +
> +       retval = socketpair(domain, type, protocol, sv);
> +       if (set_cloexec_or_close(sv[0]) < 0 || set_cloexec_or_close(sv[1]) < 0)
> +               retval = -1;
> +
> +       return retval;
> +}

This stuff does not seem to belong in this patch.

> +
>  int
>  wl_os_dupfd_cloexec(int fd, long minfd)
>  {
>  	int newfd;
>  
> +#ifdef F_DUPFD_CLOEXEC
>  	newfd = fcntl(fd, F_DUPFD_CLOEXEC, minfd);
>  	if (newfd >= 0)
>  		return newfd;
>  	if (errno != EINVAL)
>  		return -1;
> +#endif

Neither this.

>  
>  	newfd = fcntl(fd, F_DUPFD, minfd);
>  	return set_cloexec_or_close(newfd);
> @@ -123,17 +153,20 @@ wl_os_recvmsg_cloexec(int sockfd, struct msghdr *msg, int flags)
>  {
>  	ssize_t len;
>  
> +#ifdef MSG_CMSG_CLOEXEC
>  	len = recvmsg(sockfd, msg, flags | MSG_CMSG_CLOEXEC);
>  	if (len >= 0)
>  		return len;
>  	if (errno != EINVAL)
>  		return -1;
> +#endif

Nor this.

>  
>  	return recvmsg_cloexec_fallback(sockfd, msg, flags);
>  }
>  
> +#if defined(HAVE_SYS_EPOLL_H)
>  int
> -wl_os_epoll_create_cloexec(void)
> +wl_os_event_create_cloexec(void)
>  {
>  	int fd;
>  
> @@ -148,6 +181,16 @@ wl_os_epoll_create_cloexec(void)
>  	fd = epoll_create(1);
>  	return set_cloexec_or_close(fd);
>  }
> +#elif defined(HAVE_SYS_EVENT_H)
> +int
> +wl_os_event_create_cloexec(void)
> +{
> +	int fd;
> +
> +	fd = kqueue();
> +	return set_cloexec_or_close(fd);
> +}
> +#endif

Is kqueue() really incapable of creating a close-on-exec file
descriptor from the start?

I think it might be better to just keep wl_os_epoll_create_cloexec()
named as is, and have an alternative wl_os_kqueue_create_cloexec().
They will be used from the different .c files which always match which
function is provided.

>  
>  int
>  wl_os_accept_cloexec(int sockfd, struct sockaddr *addr, socklen_t *addrlen)
> diff --git a/src/wayland-os.h b/src/wayland-os.h
> index f51efaa..86685e0 100644
> --- a/src/wayland-os.h
> +++ b/src/wayland-os.h
> @@ -36,7 +36,7 @@ ssize_t
>  wl_os_recvmsg_cloexec(int sockfd, struct msghdr *msg, int flags);
>  
>  int
> -wl_os_epoll_create_cloexec(void);
> +wl_os_event_create_cloexec(void);
>  
>  int
>  wl_os_accept_cloexec(int sockfd, struct sockaddr *addr, socklen_t *addrlen);
> diff --git a/tests/os-wrappers-test.c b/tests/os-wrappers-test.c
> index 102622c..e7bcd6c 100644
> --- a/tests/os-wrappers-test.c
> +++ b/tests/os-wrappers-test.c
> @@ -26,6 +26,7 @@
>  
>  #define _GNU_SOURCE
>  
> +#include <err.h>
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <assert.h>
> @@ -38,7 +39,11 @@
>  #include <stdarg.h>
>  #include <fcntl.h>
>  #include <stdio.h>
> -#include <sys/epoll.h>
> +#ifdef HAVE_SYS_EPOLL_H
> +# include <sys/epoll.h>
> +#else
> +# include <sys/event.h>
> +#endif
>  
>  #include "wayland-private.h"
>  #include "test-runner.h"
> @@ -55,8 +60,15 @@ static int wrapped_calls_fcntl;
>  static ssize_t (*real_recvmsg)(int, struct msghdr *, int);
>  static int wrapped_calls_recvmsg;
>  
> +#ifdef HAVE_SYS_EPOLL_H
>  static int (*real_epoll_create1)(int);
>  static int wrapped_calls_epoll_create1;
> +#endif
> +
> +#if HAVE_SYS_EVENT_H
> +static int (*real_kqueue)(void);
> +static int wrapped_calls_kqueue;
> +#endif
>  
>  static void
>  init_fallbacks(int do_fallbacks)
> @@ -65,7 +77,12 @@ init_fallbacks(int do_fallbacks)
>  	real_socket = dlsym(RTLD_NEXT, "socket");
>  	real_fcntl = dlsym(RTLD_NEXT, "fcntl");
>  	real_recvmsg = dlsym(RTLD_NEXT, "recvmsg");
> +#ifdef HAVE_SYS_EPOLL_H
>  	real_epoll_create1 = dlsym(RTLD_NEXT, "epoll_create1");
> +#endif
> +#ifdef HAVE_SYS_EVENT_H
> +	real_kqueue = dlsym(RTLD_NEXT, "kqueue");
> +#endif
>  }

This looks ok.

>  
>  __attribute__ ((visibility("default"))) int
> @@ -81,6 +98,8 @@ socket(int domain, int type, int protocol)
>  	return real_socket(domain, type, protocol);
>  }
>  
> +/* won't work on OpenBSD, since real fcntl have to be called early */

What does this mean?
Why can we not wrap fcntl()?

If you did wl_os_kqueue_create_cloexec() as I suggested above, would
that solve this issue as well?

> +#if !defined(__OpenBSD__)
>  __attribute__ ((visibility("default"))) int
>  fcntl(int fd, int cmd, ...)
>  {
> @@ -100,6 +119,7 @@ fcntl(int fd, int cmd, ...)
>  
>  	return real_fcntl(fd, cmd, arg);
>  }
> +#endif
>  
>  __attribute__ ((visibility("default"))) ssize_t
>  recvmsg(int sockfd, struct msghdr *msg, int flags)
> @@ -114,6 +134,7 @@ recvmsg(int sockfd, struct msghdr *msg, int flags)
>  	return real_recvmsg(sockfd, msg, flags);
>  }
>  
> +#ifdef HAVE_SYS_EPOLL_H
>  __attribute__ ((visibility("default"))) int
>  epoll_create1(int flags)
>  {
> @@ -127,6 +148,23 @@ epoll_create1(int flags)
>  
>  	return real_epoll_create1(flags);
>  }
> +#endif
> +
> +#ifdef HAVE_SYS_EVENT_H
> +__attribute__ ((visibility("default"))) int
> +kqueue(void)
> +{
> +	wrapped_calls_kqueue++;
> +
> +	if (fall_back) {
> +		wrapped_calls_kqueue++; /* kqueue() not wrapped */
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
> +	return real_kqueue();
> +}

I don't think this is necessary, because if there is no kqueue()
variant that actually could do close-on-exec internally, there is no
need to force it to the fallback path: the fallback path is used always
already.

> +#endif
>  
>  static void
>  do_os_wrappers_socket_cloexec(int n)
> @@ -184,6 +222,7 @@ do_os_wrappers_dupfd_cloexec(int n)
>  	 * Must have 4 calls if falling back, but must also allow
>  	 * falling back without a forced fallback.
>  	 */
> +	warnx("wrapped_calls_fcntl is %d, should be larger than %d", wrapped_calls_fcntl, n);

Letfover from debugging?

>  	assert(wrapped_calls_fcntl > n);
>  
>  	exec_fd_leak_check(nr_fds);
> @@ -335,14 +374,14 @@ TEST(os_wrappers_recvmsg_cloexec_fallback)
>  }
>  
>  static void
> -do_os_wrappers_epoll_create_cloexec(int n)
> +do_os_wrappers_event_create_cloexec(int n)
>  {
>  	int fd;
>  	int nr_fds;
>  
>  	nr_fds = count_open_fds();
>  
> -	fd = wl_os_epoll_create_cloexec();
> +	fd = wl_os_event_create_cloexec();
>  	assert(fd >= 0);
>  
>  #ifdef EPOLL_CLOEXEC
> @@ -354,16 +393,18 @@ do_os_wrappers_epoll_create_cloexec(int n)
>  	exec_fd_leak_check(nr_fds);
>  }
>  
> -TEST(os_wrappers_epoll_create_cloexec)
> +TEST(os_wrappers_event_create_cloexec)
>  {
>  	init_fallbacks(0);
> -	do_os_wrappers_epoll_create_cloexec(1);
> +	do_os_wrappers_event_create_cloexec(1);
>  }
>  
> -TEST(os_wrappers_epoll_create_cloexec_fallback)
> +TEST(os_wrappers_event_create_cloexec_fallback)
>  {
>  	init_fallbacks(1);
> -	do_os_wrappers_epoll_create_cloexec(2);
> +	do_os_wrappers_event_create_cloexec(2);
>  }
>  
>  /* FIXME: add tests for wl_os_accept_cloexec() */
> +
> +/* FIXME: add tests for kqueue() */

What could one test of kqueue()? If nothing comes to mind that's not
already covered, no need for a FIXME.


Thanks,
pq