From fe7382fe3b3fcae9b5d05fd42744807a32b1c555 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Wed, 26 Aug 2009 15:48:14 -0400 Subject: [PATCH] Ensure that fds are only added once in the sbus There were cases that we we were mishandling where D-BUS would try to send us the same file descriptor when changing events. Instead of trying to blindly add a new event (which caused an EEXIST error in epoll), we will remove the old tevent_fd and add the new one. Also, we were trying to be too fancy in the toggle function. It is simpler to just remove or add watches as appropriate. --- server/sbus/sssd_dbus_common.c | 123 ++++++++++++++++++++++----------------- server/sbus/sssd_dbus_private.h | 7 ++ 2 files changed, 76 insertions(+), 54 deletions(-) diff --git a/server/sbus/sssd_dbus_common.c b/server/sbus/sssd_dbus_common.c index 76cb3c2..758221c 100644 --- a/server/sbus/sssd_dbus_common.c +++ b/server/sbus/sssd_dbus_common.c @@ -50,6 +50,34 @@ static void sbus_watch_handler(struct tevent_context *ev, } } +static bool fd_in_use(struct sbus_connection *conn, int fd, + struct sbus_watch_ctx **watch) +{ + struct sbus_watch_ctx *watch_iter; + + watch_iter = conn->watch_list; + while (watch_iter != NULL) { + if (watch_iter->fd == fd) { + *watch = watch_iter; + return 1; + } + + watch_iter = watch_iter->next; + } + + return 0; +} + +static int sbus_watch_destructor(void *ctx) +{ + struct sbus_watch_ctx *watch = + talloc_get_type(ctx, struct sbus_watch_ctx); + + DLIST_REMOVE(watch->conn->watch_list, watch); + + return EOK; +} + /* * add_watch * Set up hooks into the libevents mainloop for @@ -63,15 +91,12 @@ dbus_bool_t sbus_add_watch(DBusWatch *dbus_watch, void *data) struct sbus_watch_ctx *watch; int fd; - conn = talloc_get_type(data, struct sbus_connection); - - watch = talloc_zero(conn, struct sbus_watch_ctx); - if (!watch) { - DEBUG(0, ("Outr of Memory!\n")); - return FALSE; + /* No idea why, but D-BUS sometimes tries to add disabled watches */ + if (!dbus_watch_get_enabled(dbus_watch)) { + return true; } - watch->dbus_watch = dbus_watch; - watch->conn = conn; + + conn = talloc_get_type(data, struct sbus_connection); #ifdef HAVE_DBUS_WATCH_GET_UNIX_FD fd = dbus_watch_get_unix_fd(dbus_watch); @@ -82,13 +107,16 @@ dbus_bool_t sbus_add_watch(DBusWatch *dbus_watch, void *data) flags = dbus_watch_get_flags(dbus_watch); event_flags = 0; - if (dbus_watch_get_enabled(dbus_watch)) { - if (flags & DBUS_WATCH_READABLE) { - event_flags |= TEVENT_FD_READ; - } - if (flags & DBUS_WATCH_WRITABLE) { - event_flags |= TEVENT_FD_WRITE; - } + if (flags & DBUS_WATCH_READABLE) { + event_flags |= TEVENT_FD_READ; + } + if (flags & DBUS_WATCH_WRITABLE) { + event_flags |= TEVENT_FD_WRITE; + } + + if (flags == 0) { + DEBUG(1, ("Cowardly refusing to add an fd with no read or write state\n")); + return true; } DEBUG(8, ("%p: %d, %d=%s/%s\n", @@ -96,14 +124,33 @@ dbus_bool_t sbus_add_watch(DBusWatch *dbus_watch, void *data) ((event_flags & TEVENT_FD_READ)?"R":"-"), ((event_flags & TEVENT_FD_WRITE)?"W":"-"))); + /* D-BUS may add a watch for an fd that already exists */ + if (fd_in_use(conn, fd, &watch)) { + sbus_remove_watch(dbus_watch, data); + } + + watch = talloc_zero(conn, struct sbus_watch_ctx); + if (!watch) { + DEBUG(0, ("Out of Memory!\n")); + return FALSE; + } + watch->dbus_watch = dbus_watch; + watch->conn = conn; + /* Add the file descriptor to the event loop */ - watch->fde = tevent_add_fd(conn->ev, - watch, fd, event_flags, - sbus_watch_handler, watch); + watch->fde = tevent_add_fd(conn->ev, watch, + fd, event_flags, + sbus_watch_handler, + watch); if (!watch->fde) { DEBUG(0, ("Failed to set up fd event!\n")); return FALSE; } + watch->fd = fd; + + DLIST_ADD(conn->watch_list, watch); + + talloc_set_destructor((TALLOC_CTX *)watch, sbus_watch_destructor); /* Save the event to the watch object so it can be removed later */ dbus_watch_set_data(dbus_watch, watch, NULL); @@ -118,44 +165,12 @@ dbus_bool_t sbus_add_watch(DBusWatch *dbus_watch, void *data) */ void sbus_toggle_watch(DBusWatch *dbus_watch, void *data) { - struct sbus_watch_ctx *watch; - uint16_t event_flags = 0; - unsigned int flags; - void *watch_data; - - watch_data = dbus_watch_get_data(dbus_watch); - watch = talloc_get_type(watch_data, struct sbus_watch_ctx); - if (!watch) { - DEBUG(0, ("Watch does not carry watch context?!\n")); - /* TODO: abort ? */ - return; - } - - flags = dbus_watch_get_flags(dbus_watch); - if (dbus_watch_get_enabled(dbus_watch)) { - if (flags & DBUS_WATCH_READABLE) { - TEVENT_FD_READABLE(watch->fde); - } - if (flags & DBUS_WATCH_WRITABLE) { - TEVENT_FD_WRITEABLE(watch->fde); - } - } else { - if (flags & DBUS_WATCH_READABLE) { - TEVENT_FD_NOT_READABLE(watch->fde); - } - if (flags & DBUS_WATCH_WRITABLE) { - TEVENT_FD_NOT_WRITEABLE(watch->fde); - } + sbus_add_watch(dbus_watch, data); } - - if (debug_level >= 8) { - event_flags = tevent_fd_get_flags(watch->fde); + else { + sbus_remove_watch(dbus_watch, data); } - DEBUG(8, ("%p: %p, %d=%s/%s\n", - dbus_watch, watch, flags, - ((event_flags & TEVENT_FD_READ)?"R":"-"), - ((event_flags & TEVENT_FD_WRITE)?"W":"-"))); } /* @@ -175,7 +190,7 @@ void sbus_remove_watch(DBusWatch *dbus_watch, void *data) dbus_watch_set_data(dbus_watch, NULL, NULL); /* Freeing the event object will remove it from the event loop */ - talloc_free(watch); + talloc_zfree(watch); } /* =Timeouts============================================================== */ diff --git a/server/sbus/sssd_dbus_private.h b/server/sbus/sssd_dbus_private.h index 79cae23..d327e6b 100644 --- a/server/sbus/sssd_dbus_private.h +++ b/server/sbus/sssd_dbus_private.h @@ -22,6 +22,9 @@ struct sbus_connection { int connection_type; int disconnect; + /* File descriptors used by this connection */ + struct sbus_watch_ctx *watch_list; + sbus_conn_destructor_fn destructor; void *pvt_data; /* Private data for this connection */ @@ -47,6 +50,10 @@ struct sbus_watch_ctx { DBusWatch *dbus_watch; struct sbus_connection *conn; struct tevent_fd *fde; + int fd; + + struct sbus_watch_ctx *prev; + struct sbus_watch_ctx *next; }; dbus_bool_t sbus_add_watch(DBusWatch *watch, void *data); -- 1.6.2.5