Date: Thu, 19 Jul 2007 20:34:16 GMT From: Fredrik Lindberg <fli@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 123751 for review Message-ID: <200707192034.l6JKYGGa040518@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=123751 Change 123751 by fli@fli_nexus on 2007/07/19 20:33:30 - Must hold event lock during initialization of an event, otherwise there is a removal race on events that are removed directly after they have been added (such as EOF on a new socket). - While here, use global locking macros and sort header files. Affected files ... .. //depot/projects/soc2007/fli-mdns_sd/mdnsd/event.c#6 edit .. //depot/projects/soc2007/fli-mdns_sd/mdnsd/event.h#5 edit Differences ... ==== //depot/projects/soc2007/fli-mdns_sd/mdnsd/event.c#6 (text+ko) ==== @@ -34,28 +34,15 @@ #include <strings.h> #include <unistd.h> +#include "debug.h" #include "event.h" +#include "log.h" +#include "threads.h" #include "wqueue.h" -#include "log.h" -#include "debug.h" static int remove_event(struct eventlist *, struct event *, ev_arg *); static int event_engine(wq_arg); -#ifdef HAVE_PTHREAD -#define EV_RLOCK(ev) pthread_rwlock_rdlock(&(ev)->ev_lock) -#define EV_WLOCK(ev) pthread_rwlock_wrlock(&(ev)->ev_lock) -#define EV_UNLOCK(ev) pthread_rwlock_unlock(&(ev)->ev_lock) -#define EVL_LOCK(evl) pthread_mutex_lock(&(evl)->evl_mtx) -#define EVL_UNLOCK(evl) pthread_mutex_unlock(&(evl)->evl_mtx) -#else -#define EV_RLOCK(ev) -#define EV_WLOCK(ev) -#define EV_UNLOCK(ev) -#define EVL_LOCK(evl) -#define EVL_UNLOCK(evl) -#endif - /* * Initialize a new event list */ @@ -75,9 +62,7 @@ TAILQ_INIT(&evl->evl_events); evl->evl_kq = kq; -#ifdef HAVE_PTHREAD - pthread_mutex_init(&evl->evl_mtx, NULL); -#endif + MTX_INIT(evl, evl_mtx, NULL); MDNS_INIT_SET(evl, evl_magic); return (evl); } @@ -95,13 +80,13 @@ int empty; MDNS_INIT_ASSERT(evl, evl_magic); - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); while (!TAILQ_EMPTY(&evl->evl_events)) { ev = TAILQ_FIRST(&evl->evl_events); - EV_WLOCK(ev); + RW_WLOCK(ev, ev_lock); ev->ev_flags |= EVENT_FLAG_DYING; if (remove_event(evl, ev, &ev->ev_init_arg) == 1) { - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); } } empty = TAILQ_EMPTY(&evl->evl_events); @@ -115,7 +100,7 @@ free(evl); } else { - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); } return (!empty); @@ -153,10 +138,10 @@ break; } - EV_WLOCK(ev); + RW_WLOCK(ev, ev_lock); if ((ev->ev_flags & EVENT_FLAG_EX) && ev->ev_redo > 0) { ev->ev_redo--; - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); dprintf(DEBUG_EVENT, "Executing event again ev=%x", ev); goto again; } @@ -164,11 +149,11 @@ if (ev->ev_flags & EVENT_FLAG_DYING) { evl = ev->ev_evl; if (remove_event(evl, ev, &ev->ev_init_arg) == 1) { - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); } } else { - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); } return (0); } @@ -191,9 +176,9 @@ struct kevent kev; MDNS_INIT_ASSERT(evl, evl_magic); - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); kq = evl->evl_kq; - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); /* * We (ab)use SIGUSR1 for our own evil purposes, when an event handler @@ -217,10 +202,10 @@ if (ev == NULL) continue; - EV_WLOCK(ev); + RW_WLOCK(ev, ev_lock); if (kev.flags & EV_EOF) { if (remove_event(evl, ev, &ev->ev_init_arg) == 1) { - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); } continue; } @@ -230,7 +215,7 @@ if (ev->ev_refcnt == 0) remove_event(evl, ev, &ev->ev_init_arg); else { - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); } continue; } @@ -240,12 +225,12 @@ dprintf(DEBUG_EVENT, "Exclusive event already in progress ev=%x", ev); ev->ev_redo++; - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); continue; } ev->ev_refcnt++; - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); dprintf(DEBUG_EVENT, "Event fired, dispatched to queue=%x, ev=%x", wq, ev); wa.ptr = ev; @@ -272,10 +257,10 @@ pid_t self; MDNS_INIT_ASSERT(evl, evl_magic); - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); dprintf(DEBUG_EVENT, "Event exit called"); evl->evl_flags |= EVL_FLAG_DYING; - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); self = getpid(); kill(self, SIGUSR1); @@ -327,10 +312,11 @@ #ifdef HAVE_PTHREAD pthread_rwlock_init(&ev->ev_lock, NULL); #endif + RW_WLOCK(ev, ev_lock); - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); ev->ev_id = evl->evl_len++; - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); switch (type) { case EVENT_TYPE_IO: @@ -377,19 +363,21 @@ kev.flags |= EV_ADD | EV_CLEAR; ret = kevent(evl->evl_kq, &kev, 1, NULL, 0, NULL); if (ret != 0) { + RW_UNLOCK(ev, ev_lock); free(ev); dprintf(DEBUG_EVENT, "Failed to install kevent"); return (-1); } else { - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); TAILQ_INSERT_TAIL(&evl->evl_events, ev, ev_evlist); - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); } MDNS_INIT_SET(ev, ev_magic); dprintf(DEBUG_EVENT, "Event added type=%d, id=%d, ev=%x, handler=%x", type, ev->ev_id, ev, handler); + RW_UNLOCK(ev, ev_lock); return (ev->ev_id); } @@ -408,17 +396,17 @@ struct event *ev = NULL; MDNS_INIT_ASSERT(evl, evl_magic); - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); TAILQ_FOREACH(ev, &evl->evl_events, ev_evlist) { if (ev->ev_id == id) break; } - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); if (ev == NULL) { return (-1); } - EV_WLOCK(ev); + RW_WLOCK(ev, ev_lock); dprintf(DEBUG_EVENT, "Removing event ev=%x", ev); if (remove_event(evl, ev, arg) == 1) { @@ -429,7 +417,7 @@ * can be removed. */ memcpy(&ev->ev_init_arg, arg, sizeof(ev_arg)); - EV_UNLOCK(ev); + RW_UNLOCK(ev, ev_lock); } return (0); @@ -461,9 +449,9 @@ } bzero(&kev, sizeof(struct kevent)); - EVL_LOCK(evl); + MTX_LOCK(evl, evl_mtx); TAILQ_REMOVE(&evl->evl_events, ev, ev_evlist); - EVL_UNLOCK(evl); + MTX_UNLOCK(evl, evl_mtx); if (arg != NULL) memcpy(&ev_arg_init, arg, sizeof(ev_arg)); ==== //depot/projects/soc2007/fli-mdns_sd/mdnsd/event.h#5 (text+ko) ==== @@ -30,8 +30,10 @@ #include <stdint.h> #include <sys/queue.h> +#include "debug.h" +#include "threads.h" #include "wqueue.h" -#include "debug.h" + /* * Holds event id and backpointer to event list, included @@ -100,9 +102,7 @@ struct eventlist { MAGIC(evl_magic); TAILQ_HEAD(, event) evl_events; /* list of events */ -#ifdef HAVE_PTHREAD - pthread_mutex_t evl_mtx; -#endif + DEF_MTX(evl_mtx); int evl_len; int evl_flags; #define EVL_FLAG_DYING 0x1 @@ -116,9 +116,7 @@ MAGIC(ev_magic); TAILQ_ENTRY(event) ev_evlist; /* global event list */ struct eventlist *ev_evl; /* back-pointer to list */ -#ifdef HAVE_PTHREAD - pthread_rwlock_t ev_lock; -#endif + DEF_RW(ev_lock); int ev_refcnt; /* reference counter */ int ev_flags; #define EVENT_FLAG_EX 0x1 /* exclusive event (refcnt=1) */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200707192034.l6JKYGGa040518>