From: Simon McVittie Date: Mon, 13 Jun 2011 16:04:30 +0000 (+0100) Subject: DBusSocketSet: new abstraction for struct pollfd[] or whatever X-Git-Tag: dbus-1.5.10~71^2~6 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d3d44dd38f74129503e0623f5c9f77e01e572406;p=platform%2Fupstream%2Fdbus.git DBusSocketSet: new abstraction for struct pollfd[] or whatever In this second version of this patch, DBusSocketSet is an "abstract base class" so that when using a better OS-specific API fails, we can always fall back to _dbus_poll(). For instance, this is necessary when the "better OS-specific API" is epoll on Linux, the build machine has a modern glibc version, and the host machine either has an old kernel, is emulated in qemu (which does not support the epoll syscalls yet), or both. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=33337 Bug-NB: NB#197191 Bug-NB: NB#225019 --- diff --git a/dbus/Makefile.am b/dbus/Makefile.am index 2e6ca1d..7fb7ee2 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -239,6 +239,9 @@ DBUS_UTIL_SOURCES= \ dbus-shell.c \ dbus-shell.h \ $(DBUS_UTIL_arch_sources) \ + dbus-socket-set.h \ + dbus-socket-set.c \ + dbus-socket-set-poll.c \ dbus-spawn.h \ dbus-string-util.c \ dbus-sysdeps-util.c \ diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h index 3e2a7d8..fe4d04e 100644 --- a/dbus/dbus-connection.h +++ b/dbus/dbus-connection.h @@ -69,6 +69,7 @@ typedef enum * state passed to * dbus_watch_handle()). */ + /* Internal to libdbus, there is also _DBUS_WATCH_NVAL in dbus-watch.h */ } DBusWatchFlags; /** diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c index b49c9b2..e506b37 100644 --- a/dbus/dbus-mainloop.c +++ b/dbus/dbus-mainloop.c @@ -1,7 +1,8 @@ /* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */ /* dbus-mainloop.c Main loop utility * - * Copyright (C) 2003, 2004 Red Hat, Inc. + * Copyright © 2003, 2004 Red Hat, Inc. + * Copyright © 2011 Nokia Corporation * * Licensed under the Academic Free License version 2.1 * @@ -28,7 +29,7 @@ #include #include -#include +#include #include #define MAINLOOP_SPEW 0 @@ -67,36 +68,6 @@ struct DBusLoop DBusList *need_dispatch; }; -static short -watch_flags_to_poll_events (unsigned int flags) -{ - short events = 0; - - if (flags & DBUS_WATCH_READABLE) - events |= _DBUS_POLLIN; - if (flags & DBUS_WATCH_WRITABLE) - events |= _DBUS_POLLOUT; - - return events; -} - -static unsigned int -watch_flags_from_poll_revents (short revents) -{ - unsigned int condition = 0; - - if (revents & _DBUS_POLLIN) - condition |= DBUS_WATCH_READABLE; - if (revents & _DBUS_POLLOUT) - condition |= DBUS_WATCH_WRITABLE; - if (revents & _DBUS_POLLHUP) - condition |= DBUS_WATCH_HANGUP; - if (revents & _DBUS_POLLERR) - condition |= DBUS_WATCH_ERROR; - - return condition; -} - typedef struct { int refcount; @@ -568,9 +539,8 @@ _dbus_loop_iterate (DBusLoop *loop, { #define N_STACK_DESCRIPTORS 64 dbus_bool_t retval; - DBusPollFD *fds; - DBusPollFD stack_fds[N_STACK_DESCRIPTORS]; - int n_fds; + DBusSocketSet *socket_set = NULL; + DBusSocketEvent ready_fds[N_STACK_DESCRIPTORS]; int i; DBusList *link; int n_ready; @@ -582,8 +552,6 @@ _dbus_loop_iterate (DBusLoop *loop, retval = FALSE; - fds = NULL; - n_fds = 0; oom_watch_pending = FALSE; orig_depth = loop->depth; @@ -596,23 +564,15 @@ _dbus_loop_iterate (DBusLoop *loop, loop->timeouts == NULL) goto next_iteration; - if (loop->watch_count > N_STACK_DESCRIPTORS) + socket_set = _dbus_socket_set_new (loop->watch_count); + + while (socket_set == NULL) { - fds = dbus_new0 (DBusPollFD, loop->watch_count); - - while (fds == NULL) - { - _dbus_wait_for_memory (); - fds = dbus_new0 (DBusPollFD, loop->watch_count); - } - } - else - { - fds = stack_fds; + _dbus_wait_for_memory (); + socket_set = _dbus_socket_set_new (loop->watch_count); } /* fill our array of fds and watches */ - n_fds = 0; _dbus_hash_iter_init (loop->watches, &hash_iter); while (_dbus_hash_iter_next (&hash_iter)) @@ -620,10 +580,12 @@ _dbus_loop_iterate (DBusLoop *loop, DBusList **watches; unsigned int flags; int fd; + dbus_bool_t enabled; fd = _dbus_hash_iter_get_int_key (&hash_iter); watches = _dbus_hash_iter_get_value (&hash_iter); flags = 0; + enabled = FALSE; for (link = _dbus_list_get_first_link (watches); link != NULL; @@ -651,28 +613,24 @@ _dbus_loop_iterate (DBusLoop *loop, else if (dbus_watch_get_enabled (watch)) { flags |= dbus_watch_get_flags (watch); + enabled = TRUE; } } - if (flags != 0) + if (enabled) { - fds[n_fds].fd = fd; - fds[n_fds].revents = 0; - fds[n_fds].events = watch_flags_to_poll_events (flags); + _dbus_socket_set_add (socket_set, fd, flags, TRUE); #if MAINLOOP_SPEW _dbus_verbose (" polling watch on fd %d %s\n", - loop->fds[loop->n_fds].fd, watch_flags_to_string (flags)); + fd, watch_flags_to_string (flags)); #endif - - n_fds += 1; } else { #if MAINLOOP_SPEW _dbus_verbose (" skipping disabled watch on fd %d %s\n", - fd, - watch_flags_to_string (dbus_watch_get_flags (watch))); + fd, watch_flags_to_string (flags)); #endif } } @@ -741,8 +699,9 @@ _dbus_loop_iterate (DBusLoop *loop, #if MAINLOOP_SPEW _dbus_verbose (" polling on %d descriptors timeout %ld\n", n_fds, timeout); #endif - - n_ready = _dbus_poll (fds, n_fds, timeout); + + n_ready = _dbus_socket_set_poll (socket_set, ready_fds, + _DBUS_N_ELEMENTS (ready_fds), timeout); initial_serial = loop->callback_list_serial; @@ -805,10 +764,10 @@ _dbus_loop_iterate (DBusLoop *loop, link = next; } } - + if (n_ready > 0) { - for (i = 0; i < n_fds; i++) + for (i = 0; i < n_ready; i++) { DBusList **watches; DBusList *next; @@ -824,16 +783,16 @@ _dbus_loop_iterate (DBusLoop *loop, if (loop->depth != orig_depth) goto next_iteration; - if (fds[i].revents == 0) - continue; + _dbus_assert (ready_fds[i].flags != 0); - if (_DBUS_UNLIKELY (fds[i].revents & _DBUS_POLLNVAL)) + if (_DBUS_UNLIKELY (ready_fds[i].flags & _DBUS_WATCH_NVAL)) { - cull_watches_for_invalid_fd (loop, fds[i].fd); + cull_watches_for_invalid_fd (loop, ready_fds[i].fd); goto next_iteration; } - condition = watch_flags_from_poll_revents (fds[i].revents); + condition = ready_fds[i].flags; + _dbus_assert ((condition & _DBUS_WATCH_NVAL) == 0); /* condition may still be 0 if we got some * weird POLLFOO thing like POLLWRBAND @@ -841,7 +800,8 @@ _dbus_loop_iterate (DBusLoop *loop, if (condition == 0) continue; - watches = _dbus_hash_table_lookup_int (loop->watches, fds[i].fd); + watches = _dbus_hash_table_lookup_int (loop->watches, + ready_fds[i].fd); if (watches == NULL) continue; @@ -887,9 +847,9 @@ _dbus_loop_iterate (DBusLoop *loop, #if MAINLOOP_SPEW _dbus_verbose (" moving to next iteration\n"); #endif - - if (fds && fds != stack_fds) - dbus_free (fds); + + if (socket_set) + _dbus_socket_set_free (socket_set); if (_dbus_loop_dispatch (loop)) retval = TRUE; diff --git a/dbus/dbus-socket-set-poll.c b/dbus/dbus-socket-set-poll.c new file mode 100644 index 0000000..65a1fd2 --- /dev/null +++ b/dbus/dbus-socket-set-poll.c @@ -0,0 +1,320 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */ +/* dbus-socket-set-poll.c - a socket set implemented via _dbus_poll + * + * Copyright © 2011 Nokia Corporation + * + * Licensed under the Academic Free License version 2.1 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + * + */ + +#include +#include "dbus-socket-set.h" + +#include +#include +#include +#include + +#ifndef DOXYGEN_SHOULD_SKIP_THIS + +typedef struct { + DBusSocketSet parent; + DBusPollFD *fds; + int n_fds; + int n_reserved; + int n_allocated; +} DBusSocketSetPoll; + +#define REALLOC_INCREMENT 8 +#define MINIMUM_SIZE 8 + +/* If we're in the regression tests, force reallocation to happen sooner */ +#ifdef DBUS_BUILD_TESTS +#define DEFAULT_SIZE_HINT 1 +#else +#define DEFAULT_SIZE_HINT MINIMUM_SIZE +#endif + +static inline DBusSocketSetPoll * +socket_set_poll_cast (DBusSocketSet *set) +{ + _dbus_assert (set->cls == &_dbus_socket_set_poll_class); + return (DBusSocketSetPoll *) set; +} + +/* this is safe to call on a partially-allocated socket set */ +static void +socket_set_poll_free (DBusSocketSet *set) +{ + DBusSocketSetPoll *self = socket_set_poll_cast (set); + + dbus_free (self->fds); + dbus_free (self); + _dbus_verbose ("freed socket set %p\n", self); +} + +DBusSocketSet * +_dbus_socket_set_poll_new (int size_hint) +{ + DBusSocketSetPoll *ret; + + if (size_hint <= 0) + size_hint = DEFAULT_SIZE_HINT; + + ret = dbus_new0 (DBusSocketSetPoll, 1); + + if (ret == NULL) + return NULL; + + ret->parent.cls = &_dbus_socket_set_poll_class; + ret->n_fds = 0; + ret->n_allocated = size_hint; + + ret->fds = dbus_new0 (DBusPollFD, size_hint); + + if (ret->fds == NULL) + { + /* socket_set_poll_free specifically supports half-constructed + * socket sets */ + socket_set_poll_free ((DBusSocketSet *) ret); + return NULL; + } + + _dbus_verbose ("new socket set at %p\n", ret); + return (DBusSocketSet *) ret; +} + +static short +watch_flags_to_poll_events (unsigned int flags) +{ + short events = 0; + + if (flags & DBUS_WATCH_READABLE) + events |= _DBUS_POLLIN; + if (flags & DBUS_WATCH_WRITABLE) + events |= _DBUS_POLLOUT; + + return events; +} + +static dbus_bool_t +socket_set_poll_add (DBusSocketSet *set, + int fd, + unsigned int flags, + dbus_bool_t enabled) +{ + DBusSocketSetPoll *self = socket_set_poll_cast (set); +#ifndef DBUS_DISABLE_ASSERT + int i; + + for (i = 0; i < self->n_fds; i++) + _dbus_assert (self->fds[i].fd != fd); +#endif + + if (self->n_reserved >= self->n_allocated) + { + DBusPollFD *new_fds = dbus_realloc (self->fds, + sizeof (DBusPollFD) * (self->n_allocated + REALLOC_INCREMENT)); + + _dbus_verbose ("inflating set %p from %d en/%d res/%d alloc to %d\n", + self, self->n_fds, self->n_reserved, self->n_allocated, + self->n_allocated + REALLOC_INCREMENT); + + if (new_fds == NULL) + return FALSE; + + self->fds = new_fds; + self->n_allocated += REALLOC_INCREMENT; + } + + _dbus_verbose ("before adding fd %d to %p, %d en/%d res/%d alloc\n", + fd, self, self->n_fds, self->n_reserved, self->n_allocated); + _dbus_assert (self->n_reserved >= self->n_fds); + _dbus_assert (self->n_allocated > self->n_reserved); + + self->n_reserved++; + + if (enabled) + { + self->fds[self->n_fds].fd = fd; + self->fds[self->n_fds].events = watch_flags_to_poll_events (flags); + self->n_fds++; + } + + return TRUE; +} + +static void +socket_set_poll_enable (DBusSocketSet *set, + int fd, + unsigned int flags) +{ + DBusSocketSetPoll *self = socket_set_poll_cast (set); + int i; + + for (i = 0; i < self->n_fds; i++) + { + if (self->fds[i].fd == fd) + { + self->fds[i].events = watch_flags_to_poll_events (flags); + return; + } + } + + /* we allocated space when the socket was added */ + _dbus_assert (self->n_fds < self->n_reserved); + _dbus_assert (self->n_reserved <= self->n_allocated); + + self->fds[self->n_fds].fd = fd; + self->fds[self->n_fds].events = watch_flags_to_poll_events (flags); + self->n_fds++; +} + +static void +socket_set_poll_disable (DBusSocketSet *set, + int fd) +{ + DBusSocketSetPoll *self = socket_set_poll_cast (set); + int i; + + for (i = 0; i < self->n_fds; i++) + { + if (self->fds[i].fd == fd) + { + if (i != self->n_fds - 1) + { + self->fds[i].fd = self->fds[self->n_fds - 1].fd; + self->fds[i].events = self->fds[self->n_fds - 1].events; + } + + self->n_fds--; + return; + } + } +} + +static void +socket_set_poll_remove (DBusSocketSet *set, + int fd) +{ + DBusSocketSetPoll *self = socket_set_poll_cast (set); + + socket_set_poll_disable (set, fd); + self->n_reserved--; + + _dbus_verbose ("after removing fd %d from %p, %d en/%d res/%d alloc\n", + fd, self, self->n_fds, self->n_reserved, self->n_allocated); + _dbus_assert (self->n_fds <= self->n_reserved); + _dbus_assert (self->n_reserved <= self->n_allocated); + + if (self->n_reserved + MINIMUM_SIZE < self->n_allocated / 2) + { + /* Our array is twice as big as it needs to be - deflate it until it's + * only slightly larger than the number reserved. */ + DBusPollFD *new_fds = dbus_realloc (self->fds, + sizeof (DBusPollFD) * (self->n_reserved + MINIMUM_SIZE)); + + _dbus_verbose ("before deflating %p, %d en/%d res/%d alloc\n", + self, self->n_fds, self->n_reserved, self->n_allocated); + + if (_DBUS_UNLIKELY (new_fds == NULL)) + { + /* Weird. Oh well, never mind, the too-big array is untouched */ + return; + } + + self->fds = new_fds; + self->n_allocated = self->n_reserved; + } +} + +static unsigned int +watch_flags_from_poll_revents (short revents) +{ + unsigned int condition = 0; + + if (revents & _DBUS_POLLIN) + condition |= DBUS_WATCH_READABLE; + if (revents & _DBUS_POLLOUT) + condition |= DBUS_WATCH_WRITABLE; + if (revents & _DBUS_POLLHUP) + condition |= DBUS_WATCH_HANGUP; + if (revents & _DBUS_POLLERR) + condition |= DBUS_WATCH_ERROR; + + if (_DBUS_UNLIKELY (revents & _DBUS_POLLNVAL)) + condition |= _DBUS_WATCH_NVAL; + + return condition; +} + +/** This is basically Linux's epoll_wait(2) implemented in terms of poll(2); + * it returns results into a caller-supplied buffer so we can be reentrant. */ +static int +socket_set_poll_poll (DBusSocketSet *set, + DBusSocketEvent *revents, + int max_events, + int timeout_ms) +{ + DBusSocketSetPoll *self = socket_set_poll_cast (set); + int i; + int n_events; + int n_ready; + + _dbus_assert (max_events > 0); + + for (i = 0; i < self->n_fds; i++) + self->fds[i].revents = 0; + + n_ready = _dbus_poll (self->fds, self->n_fds, timeout_ms); + + if (n_ready <= 0) + return n_ready; + + n_events = 0; + + for (i = 0; i < self->n_fds; i++) + { + if (self->fds[i].revents != 0) + { + revents[n_events].fd = self->fds[i].fd; + revents[n_events].flags = watch_flags_from_poll_revents (self->fds[i].revents); + + n_events += 1; + + /* We ignore events beyond max_events because we have nowhere to + * put them. _dbus_poll is level-triggered, so we'll just be told + * about them next time round the main loop anyway. */ + if (n_events == max_events) + return n_events; + } + } + + return n_events; +} + +DBusSocketSetClass _dbus_socket_set_poll_class = { + socket_set_poll_free, + socket_set_poll_add, + socket_set_poll_remove, + socket_set_poll_enable, + socket_set_poll_disable, + socket_set_poll_poll +}; + +#endif /* !DOXYGEN_SHOULD_SKIP_THIS */ diff --git a/dbus/dbus-socket-set.c b/dbus/dbus-socket-set.c new file mode 100644 index 0000000..ddc4751 --- /dev/null +++ b/dbus/dbus-socket-set.c @@ -0,0 +1,40 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */ +/* + * dbus-socket-set.c - used to bolt file descriptors onto a bus + * + * Copyright © 2011 Nokia Corporation + * + * Licensed under the Academic Free License version 2.1 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + * + */ + +#include +#include + +DBusSocketSet * +_dbus_socket_set_new (int size_hint) +{ + DBusSocketSet *ret; + + ret = _dbus_socket_set_poll_new (size_hint); + + if (ret != NULL) + return ret; + + return NULL; +} diff --git a/dbus/dbus-socket-set.h b/dbus/dbus-socket-set.h new file mode 100644 index 0000000..aca17da --- /dev/null +++ b/dbus/dbus-socket-set.h @@ -0,0 +1,120 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */ +/* + * dbus-socket-set.h - used to bolt file descriptors onto a bus + * + * Copyright © 2011 Nokia Corporation + * + * Licensed under the Academic Free License version 2.1 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + * + */ + +#ifndef DBUS_SOCKET_SET_H +#define DBUS_SOCKET_SET_H + +#ifndef DOXYGEN_SHOULD_SKIP_THIS + +#include + +typedef struct { + int fd; + unsigned int flags; +} DBusSocketEvent; + +typedef struct DBusSocketSet DBusSocketSet; + +typedef struct DBusSocketSetClass DBusSocketSetClass; +struct DBusSocketSetClass { + void (*free) (DBusSocketSet *self); + dbus_bool_t (*add) (DBusSocketSet *self, + int fd, + unsigned int flags, + dbus_bool_t enabled); + void (*remove) (DBusSocketSet *self, + int fd); + void (*enable) (DBusSocketSet *self, + int fd, + unsigned int flags); + void (*disable) (DBusSocketSet *self, + int fd); + int (*poll) (DBusSocketSet *self, + DBusSocketEvent *revents, + int max_events, + int timeout_ms); +}; + +struct DBusSocketSet { + DBusSocketSetClass *cls; +}; + +DBusSocketSet *_dbus_socket_set_new (int size_hint); + +static inline void +_dbus_socket_set_free (DBusSocketSet *self) +{ + (self->cls->free) (self); +} + +static inline dbus_bool_t +_dbus_socket_set_add (DBusSocketSet *self, + int fd, + unsigned int flags, + dbus_bool_t enabled) +{ + return (self->cls->add) (self, fd, flags, enabled); +} + +static inline void +_dbus_socket_set_remove (DBusSocketSet *self, + int fd) +{ + (self->cls->remove) (self, fd); +} + +static inline void +_dbus_socket_set_enable (DBusSocketSet *self, + int fd, + unsigned int flags) +{ + (self->cls->enable) (self, fd, flags); +} + +static inline void +_dbus_socket_set_disable (DBusSocketSet *self, + int fd) +{ + (self->cls->disable) (self, fd); +} + + +static inline int +_dbus_socket_set_poll (DBusSocketSet *self, + DBusSocketEvent *revents, + int max_events, + int timeout_ms) +{ + return (self->cls->poll) (self, revents, max_events, timeout_ms); +} + +/* concrete implementations, not necessarily built on all platforms */ + +extern DBusSocketSetClass _dbus_socket_set_poll_class; + +DBusSocketSet *_dbus_socket_set_poll_new (int size_hint); + +#endif /* !DOXYGEN_SHOULD_SKIP_THIS */ +#endif /* multiple-inclusion guard */ diff --git a/dbus/dbus-watch.h b/dbus/dbus-watch.h index dd23b86..c583214 100644 --- a/dbus/dbus-watch.h +++ b/dbus/dbus-watch.h @@ -37,6 +37,8 @@ DBUS_BEGIN_DECLS typedef struct DBusWatchList DBusWatchList; +#define _DBUS_WATCH_NVAL (1<<4) + /** function to run when the watch is handled */ typedef dbus_bool_t (* DBusWatchHandler) (DBusWatch *watch, unsigned int flags,