From 76cd2b70bb64962636cba0073571d0694b9c237a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 27 Nov 2009 16:44:47 -0500 Subject: [PATCH] Stop passing EVTHREAD_READ and EVTHREAD_WRITE to non-rw locks. Previously, our default lock model kind of assumed that every lock was potentially a read-write lock. This was a poor choice, since read-write locks are far more expensive than regular locks, and so the lock API should only use them when we can actually take advantage of them. Neither our pthreads or win32 lock implementation provided rw locks. Now that we have a way (not currently used!) to indicate that we really want a read-write lock, we shouldn't actually say "lock this for reading" or "lock this for writing" unless we mean it. --- buffer.c | 148 ++++++++++++++++++++--------------------- buffer_iocp.c | 16 ++--- bufferevent-internal.h | 4 +- devpoll.c | 4 +- epoll.c | 4 +- evbuffer-internal.h | 14 ++-- evdns.c | 6 +- event.c | 49 +++++++------- evport.c | 4 +- evthread-internal.h | 14 ++-- evthread.c | 20 +++++- kqueue.c | 4 +- poll.c | 4 +- select.c | 4 +- win32select.c | 4 +- 15 files changed, 155 insertions(+), 144 deletions(-) diff --git a/buffer.c b/buffer.c index a184c8dc5e..4111a8e43c 100644 --- a/buffer.c +++ b/buffer.c @@ -277,27 +277,27 @@ evbuffer_new(void) void _evbuffer_incref(struct evbuffer *buf) { - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); ++buf->refcnt; - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); } void _evbuffer_incref_and_lock(struct evbuffer *buf) { - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); ++buf->refcnt; } int evbuffer_defer_callbacks(struct evbuffer *buffer, struct event_base *base) { - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); buffer->cb_queue = event_base_get_deferred_cb_queue(base); buffer->deferred_cbs = 1; event_deferred_cb_init(&buffer->deferred, evbuffer_deferred_callback, buffer); - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return 0; } @@ -328,9 +328,9 @@ evbuffer_enable_locking(struct evbuffer *buf, void *lock) void evbuffer_set_parent(struct evbuffer *buf, struct bufferevent *bev) { - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); buf->parent = bev; - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); } static void @@ -386,7 +386,7 @@ evbuffer_invoke_callbacks(struct evbuffer *buffer) _evbuffer_incref_and_lock(buffer); if (buffer->parent) bufferevent_incref(buffer->parent); - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); event_deferred_cb_schedule(buffer->cb_queue, &buffer->deferred); } else { evbuffer_run_callbacks(buffer); @@ -401,7 +401,7 @@ evbuffer_deferred_callback(struct deferred_cb *cb, void *arg) /* XXXX It would be better to run these callbacks without holding the * lock */ - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); parent = buffer->parent; evbuffer_run_callbacks(buffer); _evbuffer_decref_and_unlock(buffer); @@ -427,7 +427,7 @@ _evbuffer_decref_and_unlock(struct evbuffer *buffer) ASSERT_EVBUFFER_LOCKED(buffer); if (--buffer->refcnt > 0) { - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return; } @@ -439,7 +439,7 @@ _evbuffer_decref_and_unlock(struct evbuffer *buffer) if (buffer->deferred_cbs) event_deferred_cb_cancel(buffer->cb_queue, &buffer->deferred); - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); if (buffer->own_lock) EVTHREAD_FREE_LOCK(buffer->lock, EVTHREAD_LOCKTYPE_RECURSIVE); mm_free(buffer); @@ -448,20 +448,20 @@ _evbuffer_decref_and_unlock(struct evbuffer *buffer) void evbuffer_free(struct evbuffer *buffer) { - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); _evbuffer_decref_and_unlock(buffer); } void evbuffer_lock(struct evbuffer *buf) { - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); } void evbuffer_unlock(struct evbuffer *buf) { - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); } size_t @@ -469,11 +469,11 @@ evbuffer_get_length(const struct evbuffer *buffer) { size_t result; - EVBUFFER_LOCK(buffer, EVTHREAD_READ); + EVBUFFER_LOCK(buffer); result = (buffer->total_len); - EVBUFFER_UNLOCK(buffer, EVTHREAD_READ); + EVBUFFER_UNLOCK(buffer); return result; } @@ -484,10 +484,10 @@ evbuffer_get_contiguous_space(const struct evbuffer *buf) struct evbuffer_chain *chain; size_t result; - EVBUFFER_LOCK(buf, EVTHREAD_READ); + EVBUFFER_LOCK(buf); chain = buf->first; result = (chain != NULL ? chain->off : 0); - EVBUFFER_UNLOCK(buf, EVTHREAD_READ); + EVBUFFER_UNLOCK(buf); return result; } @@ -499,7 +499,7 @@ evbuffer_reserve_space(struct evbuffer *buf, ev_ssize_t size, struct evbuffer_chain *chain; int n = -1; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); if (buf->freeze_end) goto done; if (n_vecs < 1) @@ -519,7 +519,7 @@ evbuffer_reserve_space(struct evbuffer *buf, ev_ssize_t size, } done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return n; } @@ -534,7 +534,7 @@ evbuffer_commit_space(struct evbuffer *buf, size_t added; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); if (buf->freeze_end) goto done; if (n_vecs < 1 || n_vecs > 2) @@ -572,7 +572,7 @@ evbuffer_commit_space(struct evbuffer *buf, evbuffer_invoke_callbacks(buf); done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -695,7 +695,7 @@ evbuffer_drain(struct evbuffer *buf, size_t len) size_t old_len; int result = 0; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); old_len = buf->total_len; if (old_len == 0) @@ -743,7 +743,7 @@ evbuffer_drain(struct evbuffer *buf, size_t len) evbuffer_invoke_callbacks(buf); done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -758,7 +758,7 @@ evbuffer_remove(struct evbuffer *buf, void *data_out, size_t datlen) size_t nread; int result = 0; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); chain = buf->first; @@ -805,7 +805,7 @@ evbuffer_remove(struct evbuffer *buf, void *data_out, size_t datlen) result = nread; done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -899,7 +899,7 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size) unsigned char *buffer, *result = NULL; ev_ssize_t remaining; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); chain = buf->first; @@ -988,7 +988,7 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size) result = (tmp->buffer + tmp->misalign); done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -1109,7 +1109,7 @@ evbuffer_search_eol(struct evbuffer *buffer, size_t extra_drain = 0; int ok = 0; - EVBUFFER_LOCK(buffer, EVTHREAD_READ); + EVBUFFER_LOCK(buffer); if (start) { memcpy(&it, start, sizeof(it)); @@ -1164,7 +1164,7 @@ evbuffer_search_eol(struct evbuffer *buffer, ok = 1; done: - EVBUFFER_UNLOCK(buffer, EVTHREAD_READ); + EVBUFFER_UNLOCK(buffer); if (!ok) { it.pos = -1; @@ -1184,7 +1184,7 @@ evbuffer_readln(struct evbuffer *buffer, size_t *n_read_out, size_t n_to_copy=0, extra_drain=0; char *result = NULL; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); if (buffer->freeze_start) { goto done; @@ -1206,7 +1206,7 @@ evbuffer_readln(struct evbuffer *buffer, size_t *n_read_out, evbuffer_drain(buffer, extra_drain); result = line; done: - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); if (n_read_out) *n_read_out = result ? n_to_copy : 0; @@ -1226,7 +1226,7 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen) size_t remain, to_alloc; int result = -1; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); if (buf->freeze_end) { goto done; @@ -1297,7 +1297,7 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen) evbuffer_invoke_callbacks(buf); result = 0; done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -1307,7 +1307,7 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen) struct evbuffer_chain *chain, *tmp; int result = -1; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); if (buf->freeze_start) { goto done; @@ -1364,7 +1364,7 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen) evbuffer_invoke_callbacks(buf); result = 0; done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -1389,7 +1389,7 @@ evbuffer_expand(struct evbuffer *buf, size_t datlen) size_t need, length; int result = -1; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); chain = buf->last; @@ -1440,7 +1440,7 @@ evbuffer_expand(struct evbuffer *buf, size_t datlen) ok: result = 0; err: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -1622,7 +1622,7 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch) long lng = n; #endif - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); if (buf->freeze_end) { result = -1; @@ -1750,7 +1750,7 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch) evbuffer_invoke_callbacks(buf); result = n; done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -1856,7 +1856,7 @@ evbuffer_write_atmost(struct evbuffer *buffer, evutil_socket_t fd, { int n = -1; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); if (buffer->freeze_start) { goto done; @@ -1889,7 +1889,7 @@ evbuffer_write_atmost(struct evbuffer *buffer, evutil_socket_t fd, evbuffer_drain(buffer, n); done: - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return (n); } @@ -1905,7 +1905,7 @@ evbuffer_find(struct evbuffer *buffer, const unsigned char *what, size_t len) unsigned char *search; struct evbuffer_ptr ptr; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); ptr = evbuffer_search(buffer, (const char *)what, len, NULL); if (ptr.pos < 0) { @@ -1915,7 +1915,7 @@ evbuffer_find(struct evbuffer *buffer, const unsigned char *what, size_t len) if (search) search += ptr.pos; } - EVBUFFER_UNLOCK(buffer,EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return search; } @@ -1926,7 +1926,7 @@ evbuffer_ptr_set(struct evbuffer *buf, struct evbuffer_ptr *pos, size_t left = position; struct evbuffer_chain *chain = NULL; - EVBUFFER_LOCK(buf, EVTHREAD_READ); + EVBUFFER_LOCK(buf); switch (how) { case EVBUFFER_PTR_SET: @@ -1956,7 +1956,7 @@ evbuffer_ptr_set(struct evbuffer *buf, struct evbuffer_ptr *pos, pos->pos = -1; } - EVBUFFER_UNLOCK(buf, EVTHREAD_READ); + EVBUFFER_UNLOCK(buf); return chain != NULL ? 0 : -1; } @@ -2013,7 +2013,7 @@ evbuffer_search_range(struct evbuffer *buffer, const char *what, size_t len, con const unsigned char *p; char first; - EVBUFFER_LOCK(buffer, EVTHREAD_READ); + EVBUFFER_LOCK(buffer); if (start) { memcpy(&pos, start, sizeof(pos)); @@ -2066,7 +2066,7 @@ evbuffer_search_range(struct evbuffer *buffer, const char *what, size_t len, con pos.pos = -1; pos._internal.chain = NULL; done: - EVBUFFER_UNLOCK(buffer, EVTHREAD_READ); + EVBUFFER_UNLOCK(buffer); return pos; } @@ -2079,7 +2079,7 @@ evbuffer_peek(struct evbuffer *buffer, ev_ssize_t len, int idx = 0; ev_ssize_t len_so_far = 0; - EVBUFFER_LOCK(buffer, EVTHREAD_READ); + EVBUFFER_LOCK(buffer); if (start_at) { chain = start_at->_internal.chain; @@ -2109,7 +2109,7 @@ evbuffer_peek(struct evbuffer *buffer, ev_ssize_t len, chain = chain->next; } - EVBUFFER_UNLOCK(buffer, EVTHREAD_READ); + EVBUFFER_UNLOCK(buffer); return idx; } @@ -2123,7 +2123,7 @@ evbuffer_add_vprintf(struct evbuffer *buf, const char *fmt, va_list ap) int sz, result = -1; va_list aq; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); if (buf->freeze_end) { goto done; @@ -2166,7 +2166,7 @@ evbuffer_add_vprintf(struct evbuffer *buf, const char *fmt, va_list ap) /* NOTREACHED */ done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return result; } @@ -2204,7 +2204,7 @@ evbuffer_add_reference(struct evbuffer *outbuf, info->cleanupfn = cleanupfn; info->extra = extra; - EVBUFFER_LOCK(outbuf, EVTHREAD_WRITE); + EVBUFFER_LOCK(outbuf); if (outbuf->freeze_end) { /* don't call chain_free; we do not want to actually invoke * the cleanup function */ @@ -2218,7 +2218,7 @@ evbuffer_add_reference(struct evbuffer *outbuf, result = 0; done: - EVBUFFER_UNLOCK(outbuf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(outbuf); return result; } @@ -2257,7 +2257,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd, info = EVBUFFER_CHAIN_EXTRA(struct evbuffer_chain_fd, chain); info->fd = fd; - EVBUFFER_LOCK(outbuf, EVTHREAD_WRITE); + EVBUFFER_LOCK(outbuf); if (outbuf->freeze_end) { mm_free(chain); ok = 0; @@ -2304,7 +2304,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd, info = EVBUFFER_CHAIN_EXTRA(struct evbuffer_chain_fd, chain); info->fd = fd; - EVBUFFER_LOCK(outbuf, EVTHREAD_WRITE); + EVBUFFER_LOCK(outbuf); if (outbuf->freeze_end) { info->fd = -1; evbuffer_chain_free(chain); @@ -2348,7 +2348,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd, length -= read; } - EVBUFFER_LOCK(outbuf, EVTHREAD_WRITE); + EVBUFFER_LOCK(outbuf); if (outbuf->freeze_end) { evbuffer_free(tmp); ok = 0; @@ -2365,7 +2365,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd, if (ok) evbuffer_invoke_callbacks(outbuf); - EVBUFFER_UNLOCK(outbuf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(outbuf); return ok ? 0 : -1; } @@ -2374,7 +2374,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd, void evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg) { - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); if (!TAILQ_EMPTY(&buffer->callbacks)) evbuffer_remove_all_callbacks(buffer); @@ -2385,7 +2385,7 @@ evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg) ent->cb.cb_obsolete = cb; ent->flags |= EVBUFFER_CB_OBSOLETE; } - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); } struct evbuffer_cb_entry * @@ -2394,12 +2394,12 @@ evbuffer_add_cb(struct evbuffer *buffer, evbuffer_cb_func cb, void *cbarg) struct evbuffer_cb_entry *e; if (! (e = mm_calloc(1, sizeof(struct evbuffer_cb_entry)))) return NULL; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); e->cb.cb_func = cb; e->cbarg = cbarg; e->flags = EVBUFFER_CB_ENABLED; TAILQ_INSERT_HEAD(&buffer->callbacks, e, next); - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return e; } @@ -2407,9 +2407,9 @@ int evbuffer_remove_cb_entry(struct evbuffer *buffer, struct evbuffer_cb_entry *ent) { - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); TAILQ_REMOVE(&buffer->callbacks, ent, next); - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); mm_free(ent); return 0; } @@ -2419,7 +2419,7 @@ evbuffer_remove_cb(struct evbuffer *buffer, evbuffer_cb_func cb, void *cbarg) { struct evbuffer_cb_entry *cbent; int result = -1; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); TAILQ_FOREACH(cbent, &buffer->callbacks, next) { if (cb == cbent->cb.cb_func && cbarg == cbent->cbarg) { result = evbuffer_remove_cb_entry(buffer, cbent); @@ -2427,7 +2427,7 @@ evbuffer_remove_cb(struct evbuffer *buffer, evbuffer_cb_func cb, void *cbarg) } } done: - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return result; } @@ -2437,9 +2437,9 @@ evbuffer_cb_set_flags(struct evbuffer *buffer, { /* the user isn't allowed to mess with these. */ flags &= ~EVBUFFER_CB_INTERNAL_FLAGS; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); cb->flags |= flags; - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return 0; } @@ -2449,33 +2449,33 @@ evbuffer_cb_clear_flags(struct evbuffer *buffer, { /* the user isn't allowed to mess with these. */ flags &= ~EVBUFFER_CB_INTERNAL_FLAGS; - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); cb->flags &= ~flags; - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return 0; } int evbuffer_freeze(struct evbuffer *buffer, int start) { - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); if (start) buffer->freeze_start = 1; else buffer->freeze_end = 1; - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return 0; } int evbuffer_unfreeze(struct evbuffer *buffer, int start) { - EVBUFFER_LOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_LOCK(buffer); if (start) buffer->freeze_start = 0; else buffer->freeze_end = 0; - EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buffer); return 0; } diff --git a/buffer_iocp.c b/buffer_iocp.c index 7529061070..31a1d1b4dc 100644 --- a/buffer_iocp.c +++ b/buffer_iocp.c @@ -98,7 +98,7 @@ evbuffer_commit_read(struct evbuffer *evbuf, ev_ssize_t nBytes) struct evbuffer_iovec iov[2]; int n_vec; - EVBUFFER_LOCK(evbuf, EVTHREAD_WRITE); + EVBUFFER_LOCK(evbuf); EVUTIL_ASSERT(buf->read_in_progress && !buf->write_in_progress); EVUTIL_ASSERT(nBytes >= 0); // XXXX Can this be false? @@ -130,7 +130,7 @@ evbuffer_commit_write(struct evbuffer *evbuf, ev_ssize_t nBytes) { struct evbuffer_overlapped *buf = upcast_evbuffer(evbuf); - EVBUFFER_LOCK(evbuf, EVTHREAD_WRITE); + EVBUFFER_LOCK(evbuf); EVUTIL_ASSERT(buf->write_in_progress && !buf->read_in_progress); evbuffer_unfreeze(evbuf, 1); evbuffer_drain(evbuf, nBytes); @@ -170,7 +170,7 @@ evbuffer_launch_write(struct evbuffer *buf, ev_ssize_t at_most, return -1; } - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); EVUTIL_ASSERT(!buf_o->read_in_progress); if (buf->freeze_start || buf_o->write_in_progress) goto done; @@ -221,7 +221,7 @@ evbuffer_launch_write(struct evbuffer *buf, ev_ssize_t at_most, buf_o->write_in_progress = 1; r = 0; done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return r; } @@ -240,7 +240,7 @@ evbuffer_launch_read(struct evbuffer *buf, size_t at_most, if (!buf_o) return -1; - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); EVUTIL_ASSERT(!buf_o->write_in_progress); if (buf->freeze_end || buf_o->read_in_progress) goto done; @@ -286,7 +286,7 @@ evbuffer_launch_read(struct evbuffer *buf, size_t at_most, buf_o->read_in_progress = 1; r = 0; done: - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); return r; } @@ -301,9 +301,9 @@ void _evbuffer_overlapped_set_fd(struct evbuffer *buf, evutil_socket_t fd) { struct evbuffer_overlapped *buf_o = upcast_evbuffer(buf); - EVBUFFER_LOCK(buf, EVTHREAD_WRITE); + EVBUFFER_LOCK(buf); /* XXX is this right?, should it cancel current I/O operations? */ if (buf_o) buf_o->fd = fd; - EVBUFFER_UNLOCK(buf, EVTHREAD_WRITE); + EVBUFFER_UNLOCK(buf); } diff --git a/bufferevent-internal.h b/bufferevent-internal.h index 732646bd6f..937086ea62 100644 --- a/bufferevent-internal.h +++ b/bufferevent-internal.h @@ -228,14 +228,14 @@ void _bufferevent_generic_adj_timeouts(struct bufferevent *bev); #define BEV_LOCK(b) do { \ struct bufferevent_private *locking = BEV_UPCAST(b); \ if (locking->lock) \ - EVLOCK_LOCK(locking->lock, EVTHREAD_WRITE); \ + EVLOCK_LOCK(locking->lock, 0); \ } while(0) /** Internal: Release the lock (if any) on a bufferevent */ #define BEV_UNLOCK(b) do { \ struct bufferevent_private *locking = BEV_UPCAST(b); \ if (locking->lock) \ - EVLOCK_UNLOCK(locking->lock, EVTHREAD_WRITE); \ + EVLOCK_UNLOCK(locking->lock, 0); \ } while(0) #ifdef __cplusplus diff --git a/devpoll.c b/devpoll.c index 638b32d951..915fd551ab 100644 --- a/devpoll.c +++ b/devpoll.c @@ -176,11 +176,11 @@ devpoll_dispatch(struct event_base *base, struct timeval *tv) dvp.dp_nfds = devpollop->nevents; dvp.dp_timeout = timeout; - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = ioctl(devpollop->dpfd, DP_POLL, &dvp); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); if (res == -1) { if (errno != EINTR) { diff --git a/epoll.c b/epoll.c index 17215164e5..03d4bb22d6 100644 --- a/epoll.c +++ b/epoll.c @@ -145,11 +145,11 @@ epoll_dispatch(struct event_base *base, struct timeval *tv) timeout = MAX_EPOLL_TIMEOUT_MSEC; } - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = epoll_wait(epollop->epfd, events, epollop->nevents, timeout); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); if (res == -1) { if (errno != EINTR) { diff --git a/evbuffer-internal.h b/evbuffer-internal.h index 4499fec1f8..2808b6efaa 100644 --- a/evbuffer-internal.h +++ b/evbuffer-internal.h @@ -222,21 +222,20 @@ struct evbuffer_chain_reference { ((struct evbuffer*)(buffer))->lock_count--; \ } while (0) -#define EVBUFFER_LOCK(buffer, mode) \ +#define EVBUFFER_LOCK(buffer) \ do { \ - EVLOCK_LOCK((buffer)->lock, (mode)); \ + EVLOCK_LOCK((buffer)->lock, 0); \ _EVBUFFER_INCREMENT_LOCK_COUNT(buffer); \ } while(0) -#define EVBUFFER_UNLOCK(buffer, mode) \ +#define EVBUFFER_UNLOCK(buffer) \ do { \ _EVBUFFER_DECREMENT_LOCK_COUNT(buffer); \ - EVLOCK_UNLOCK((buffer)->lock, (mode)); \ + EVLOCK_UNLOCK((buffer)->lock, 0); \ } while(0) #define EVBUFFER_LOCK2(buffer1, buffer2) \ do { \ - EVLOCK_LOCK2((buffer1)->lock, (buffer2)->lock, \ - EVTHREAD_WRITE, EVTHREAD_WRITE); \ + EVLOCK_LOCK2((buffer1)->lock, (buffer2)->lock, 0, 0); \ _EVBUFFER_INCREMENT_LOCK_COUNT(buffer1); \ _EVBUFFER_INCREMENT_LOCK_COUNT(buffer2); \ } while(0) @@ -244,8 +243,7 @@ struct evbuffer_chain_reference { do { \ _EVBUFFER_DECREMENT_LOCK_COUNT(buffer1); \ _EVBUFFER_DECREMENT_LOCK_COUNT(buffer2); \ - EVLOCK_UNLOCK2((buffer1)->lock, (buffer2)->lock, \ - EVTHREAD_WRITE, EVTHREAD_WRITE); \ + EVLOCK_UNLOCK2((buffer1)->lock, (buffer2)->lock, 0, 0); \ } while(0) /** Increase the reference count of buf by one. */ diff --git a/evdns.c b/evdns.c index ce2983abd6..097dae112a 100644 --- a/evdns.c +++ b/evdns.c @@ -421,16 +421,16 @@ static int strtoint(const char *const str); #define EVDNS_LOCK(base) \ do { \ if ((base)->lock) { \ - EVLOCK_LOCK((base)->lock, EVTHREAD_WRITE); \ + EVLOCK_LOCK((base)->lock, 0); \ } \ ++(base)->lock_count; \ } while (0) #define EVDNS_UNLOCK(base) \ do { \ - EVUTIL_ASSERT((base)->lock_count > 0); \ + EVUTIL_ASSERT((base)->lock_count > 0); \ --(base)->lock_count; \ if ((base)->lock) { \ - EVLOCK_UNLOCK((base)->lock, EVTHREAD_WRITE); \ + EVLOCK_UNLOCK((base)->lock, 0); \ } \ } while (0) #define ASSERT_LOCKED(base) EVUTIL_ASSERT((base)->lock_count > 0) diff --git a/event.c b/event.c index d58416c344..aa22b3c88f 100644 --- a/event.c +++ b/event.c @@ -760,7 +760,7 @@ common_timeout_callback(evutil_socket_t fd, short what, void *arg) struct common_timeout_list *ctl = arg; struct event_base *base = ctl->base; struct event *ev = NULL; - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); gettime(base, &now); while (1) { ev = TAILQ_FIRST(&ctl->events); @@ -773,7 +773,7 @@ common_timeout_callback(evutil_socket_t fd, short what, void *arg) } if (ev) common_timeout_schedule(ctl, &now, ev); - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); } #define MAX_COMMON_TIMEOUTS 256 @@ -787,7 +787,7 @@ event_base_init_common_timeout(struct event_base *base, const struct timeval *result=NULL; struct common_timeout_list *new_ctl; - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); if (duration->tv_usec > 1000000) { memcpy(&tv, duration, sizeof(struct timeval)); if (is_common_timeout(duration, base)) @@ -848,7 +848,7 @@ event_base_init_common_timeout(struct event_base *base, if (result) EVUTIL_ASSERT(is_common_timeout(result, base)); - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); return result; } @@ -915,10 +915,9 @@ event_process_active_single_queue(struct event_base *base, base->current_event = ev; - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, current_event_lock); + EVBASE_ACQUIRE_LOCK(base, current_event_lock); - EVBASE_RELEASE_LOCK(base, - EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); switch (ev->ev_closure) { case EV_CLOSURE_SIGNAL: @@ -934,8 +933,8 @@ event_process_active_single_queue(struct event_base *base, break; } - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, current_event_lock); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, current_event_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); base->current_event = NULL; if (base->event_break) @@ -1053,9 +1052,9 @@ event_base_loopbreak(struct event_base *event_base) if (event_base == NULL) return (-1); - EVBASE_ACQUIRE_LOCK(event_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(event_base, th_base_lock); event_base->event_break = 1; - EVBASE_RELEASE_LOCK(event_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(event_base, th_base_lock); if (!EVBASE_IN_THREAD(event_base)) { return evthread_notify_base(event_base); @@ -1068,9 +1067,9 @@ int event_base_got_break(struct event_base *event_base) { int res; - EVBASE_ACQUIRE_LOCK(event_base, EVTHREAD_READ, th_base_lock); + EVBASE_ACQUIRE_LOCK(event_base, th_base_lock); res = event_base->event_break; - EVBASE_RELEASE_LOCK(event_base, EVTHREAD_READ, th_base_lock); + EVBASE_RELEASE_LOCK(event_base, th_base_lock); return res; } @@ -1078,9 +1077,9 @@ int event_base_got_exit(struct event_base *event_base) { int res; - EVBASE_ACQUIRE_LOCK(event_base, EVTHREAD_READ, th_base_lock); + EVBASE_ACQUIRE_LOCK(event_base, th_base_lock); res = event_base->event_gotterm; - EVBASE_RELEASE_LOCK(event_base, EVTHREAD_READ, th_base_lock); + EVBASE_RELEASE_LOCK(event_base, th_base_lock); return res; } @@ -1102,7 +1101,7 @@ event_base_loop(struct event_base *base, int flags) /* Grab the lock. We will release it inside evsel.dispatch, and again * as we invoke user callbacks. */ - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); clear_time_cache(base); @@ -1169,7 +1168,7 @@ event_base_loop(struct event_base *base, int flags) clear_time_cache(base); - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); event_debug(("%s: asked to terminate loop.", __func__)); return (0); @@ -1419,11 +1418,11 @@ event_add(struct event *ev, const struct timeval *tv) { int res; - EVBASE_ACQUIRE_LOCK(ev->ev_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(ev->ev_base, th_base_lock); res = event_add_internal(ev, tv, 0); - EVBASE_RELEASE_LOCK(ev->ev_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(ev->ev_base, th_base_lock); return (res); } @@ -1602,11 +1601,11 @@ event_del(struct event *ev) { int res; - EVBASE_ACQUIRE_LOCK(ev->ev_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(ev->ev_base, th_base_lock); res = event_del_internal(ev); - EVBASE_RELEASE_LOCK(ev->ev_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(ev->ev_base, th_base_lock); return (res); } @@ -1634,7 +1633,7 @@ event_del_internal(struct event *ev) base = ev->ev_base; need_cur_lock = (base->current_event == ev); if (need_cur_lock) - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, current_event_lock); + EVBASE_ACQUIRE_LOCK(base, current_event_lock); EVUTIL_ASSERT(!(ev->ev_flags & ~EVLIST_ALL)); @@ -1678,7 +1677,7 @@ event_del_internal(struct event *ev) evthread_notify_base(base); if (need_cur_lock) - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, current_event_lock); + EVBASE_RELEASE_LOCK(base, current_event_lock); return (res); } @@ -1686,11 +1685,11 @@ event_del_internal(struct event *ev) void event_active(struct event *ev, int res, short ncalls) { - EVBASE_ACQUIRE_LOCK(ev->ev_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(ev->ev_base, th_base_lock); event_active_nolock(ev, res, ncalls); - EVBASE_RELEASE_LOCK(ev->ev_base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(ev->ev_base, th_base_lock); } diff --git a/evport.c b/evport.c index f293f31b3f..1baac62116 100644 --- a/evport.c +++ b/evport.c @@ -297,12 +297,12 @@ evport_dispatch(struct event_base *base, struct timeval *tv) } } - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = port_getn(epdp->ed_port, pevtlist, EVENTS_PER_GETN, (unsigned int *) &nevents, ts_p); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); if (res == -1) { if (errno == EINTR || errno == EAGAIN) { diff --git a/evthread-internal.h b/evthread-internal.h index 623bac9e01..b7da22d46d 100644 --- a/evthread-internal.h +++ b/evthread-internal.h @@ -118,16 +118,16 @@ extern unsigned long (*_evthread_id_fn)(void); /** Lock an event_base, if it is set up for locking. Acquires the lock - in the base structure whose field is named 'lck'. */ -#define EVBASE_ACQUIRE_LOCK(base, mode, lockvar) do { \ + in the base structure whose field is named 'lockvar'. */ +#define EVBASE_ACQUIRE_LOCK(base, lockvar) do { \ if (EVBASE_USING_LOCKS(base)) \ - _evthread_lock_fns.lock(mode, (base)->lockvar); \ + _evthread_lock_fns.lock(0, (base)->lockvar); \ } while (0) /** Unlock an event_base, if it is set up for locking. */ -#define EVBASE_RELEASE_LOCK(base, mode, lockvar) do { \ +#define EVBASE_RELEASE_LOCK(base, lockvar) do { \ if (EVBASE_USING_LOCKS(base)) \ - _evthread_lock_fns.unlock(mode, (base)->lockvar); \ + _evthread_lock_fns.unlock(0, (base)->lockvar); \ } while (0) #else /* _EVENT_DISABLE_THREAD_SUPPORT */ @@ -141,8 +141,8 @@ extern unsigned long (*_evthread_id_fn)(void); #define EVLOCK_UNLOCK2(lock1,lock2,mode1,mode2) _EVUTIL_NIL_STMT #define EVBASE_IN_THREAD(base) 1 -#define EVBASE_ACQUIRE_LOCK(base, mode, lock) _EVUTIL_NIL_STMT -#define EVBASE_RELEASE_LOCK(base, mode, lock) _EVUTIL_NIL_STMT +#define EVBASE_ACQUIRE_LOCK(base, lock) _EVUTIL_NIL_STMT +#define EVBASE_RELEASE_LOCK(base, lock) _EVUTIL_NIL_STMT #endif diff --git a/evthread.c b/evthread.c index c412160f94..8222ad2792 100644 --- a/evthread.c +++ b/evthread.c @@ -38,7 +38,7 @@ #include "util-internal.h" /* globals */ -static int lock_debugging_enabled = 0; +int _evthread_lock_debugging_enabled = 0; struct evthread_lock_callbacks _evthread_lock_fns = { 0, 0, NULL, NULL, NULL, NULL }; @@ -58,7 +58,8 @@ int evthread_set_lock_callbacks(const struct evthread_lock_callbacks *cbs) { struct evthread_lock_callbacks *target = - lock_debugging_enabled ? &_original_lock_fns : &_evthread_lock_fns; + _evthread_lock_debugging_enabled + ? &_original_lock_fns : &_evthread_lock_fns; if (!cbs) { memset(target, 0, sizeof(_evthread_lock_fns)); @@ -149,6 +150,8 @@ evthread_set_lock_create_callbacks(void *(*alloc_fn)(void), struct debug_lock { unsigned locktype; + /* XXXX if we ever use read-write locks, we will need a separate + * lock to protect count. */ int count; void *lock; }; @@ -176,6 +179,7 @@ debug_lock_free(void *lock_, unsigned locktype) { struct debug_lock *lock = lock_; EVUTIL_ASSERT(lock->count == 0); + EVUTIL_ASSERT(locktype == lock->locktype); if (_original_lock_fns.free) { _original_lock_fns.free(lock->lock, lock->locktype|EVTHREAD_LOCKTYPE_RECURSIVE); @@ -190,6 +194,10 @@ debug_lock_lock(unsigned mode, void *lock_) { struct debug_lock *lock = lock_; int res = 0; + if (lock->locktype & EVTHREAD_LOCKTYPE_READWRITE) + EVUTIL_ASSERT(mode & (EVTHREAD_READ|EVTHREAD_WRITE)); + else + EVUTIL_ASSERT((mode & (EVTHREAD_READ|EVTHREAD_WRITE)) == 0); if (_original_lock_fns.lock) res = _original_lock_fns.lock(mode, lock->lock); if (!res) { @@ -205,6 +213,10 @@ debug_lock_unlock(unsigned mode, void *lock_) { struct debug_lock *lock = lock_; int res = 0; + if (lock->locktype & EVTHREAD_LOCKTYPE_READWRITE) + EVUTIL_ASSERT(mode & (EVTHREAD_READ|EVTHREAD_WRITE)); + else + EVUTIL_ASSERT((mode & (EVTHREAD_READ|EVTHREAD_WRITE)) == 0); --lock->count; EVUTIL_ASSERT(lock->count >= 0); if (_original_lock_fns.unlock) @@ -223,11 +235,13 @@ evthread_enable_lock_debuging(void) debug_lock_lock, debug_lock_unlock }; + if (_evthread_lock_debugging_enabled) + return; memcpy(&_original_lock_fns, &_evthread_lock_fns, sizeof(struct evthread_lock_callbacks)); memcpy(&_evthread_lock_fns, &cbs, sizeof(struct evthread_lock_callbacks)); - lock_debugging_enabled = 1; + _evthread_lock_debugging_enabled = 1; } #endif diff --git a/kqueue.c b/kqueue.c index 8cb7824ea8..ba9a8bc7c0 100644 --- a/kqueue.c +++ b/kqueue.c @@ -238,12 +238,12 @@ kq_dispatch(struct event_base *base, struct timeval *tv) SWAP(int, kqop->nchanges, kqop->n_pend_changes); SWAP(int, kqop->changes_size, kqop->pend_changes_size); - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = kevent(kqop->kq, kqop->pend_changes, kqop->n_pend_changes, events, kqop->events_size, ts_p); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); kqop->n_pend_changes = 0; if (res == -1) { diff --git a/poll.c b/poll.c index cbfb42f6cf..492584d086 100644 --- a/poll.c +++ b/poll.c @@ -155,11 +155,11 @@ poll_dispatch(struct event_base *base, struct timeval *tv) if (tv != NULL) msec = tv->tv_sec * 1000 + (tv->tv_usec + 999) / 1000; - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = poll(event_set, nfds, msec); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); if (res == -1) { if (errno != EINTR) { diff --git a/select.c b/select.c index 63f2b6e8f4..c0adc2eb2d 100644 --- a/select.c +++ b/select.c @@ -144,12 +144,12 @@ select_dispatch(struct event_base *base, struct timeval *tv) nfds = sop->event_fds+1; - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = select(nfds, sop->event_readset_out, sop->event_writeset_out, NULL, tv); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); check_selectop(sop); diff --git a/win32select.c b/win32select.c index d8d15f2c23..a68eba2b22 100644 --- a/win32select.c +++ b/win32select.c @@ -309,14 +309,14 @@ win32_dispatch(struct event_base *base, struct timeval *tv) return (0); } - EVBASE_RELEASE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_RELEASE_LOCK(base, th_base_lock); res = select(fd_count, (struct fd_set*)win32op->readset_out, (struct fd_set*)win32op->writeset_out, (struct fd_set*)win32op->exset_out, tv); - EVBASE_ACQUIRE_LOCK(base, EVTHREAD_WRITE, th_base_lock); + EVBASE_ACQUIRE_LOCK(base, th_base_lock); event_debug(("%s: select returned %d", __func__, res));