Permalink
Browse files

Revise the locking API: deprecate the old locking callbacks and add t…

…rylock.

Previously, there was no good way to request different kinds of lock
(say, read/write vs writeonly or recursive vs nonrecursive), or for a
lock function to signal failure (which would be important for a
trylock mode).

This patch revises the lock API to be a bit more useful.  The older
lock calls are still supported for now.

We also add a debugging mode to catch common errors in using the
locking APIs.
  • Loading branch information...
1 parent e1ffbb8 commit 347952ffe0039369ff4e7c94c4e27e7333ba8ecb Nick Mathewson committed Nov 27, 2009
Showing with 402 additions and 112 deletions.
  1. +1 −1 Makefile.am
  2. +2 −2 buffer.c
  3. +3 −2 bufferevent.c
  4. +2 −4 defer-internal.h
  5. +4 −4 evdns.c
  6. +7 −36 event.c
  7. +22 −25 evthread-internal.h
  8. +233 −0 evthread.c
  9. +27 −12 evthread_pthread.c
  10. +25 −10 evthread_win32.c
  11. +71 −16 include/event2/thread.h
  12. +5 −0 test/regress_main.c
View
@@ -107,7 +107,7 @@ event-config.h: config.h
-e 's/#ifndef /#ifndef _EVENT_/' < config.h >> $@
echo "#endif" >> $@
-CORE_SRC = event.c buffer.c \
+CORE_SRC = event.c evthread.c buffer.c \
bufferevent.c bufferevent_sock.c bufferevent_filter.c \
bufferevent_pair.c listener.c \
evmap.c log.c evutil.c strlcpy.c $(SYS_SRC)
View
@@ -311,7 +311,7 @@ evbuffer_enable_locking(struct evbuffer *buf, void *lock)
return -1;
if (!lock) {
- EVTHREAD_ALLOC_LOCK(lock);
+ EVTHREAD_ALLOC_LOCK(lock, EVTHREAD_LOCKTYPE_RECURSIVE);
if (!lock)
return -1;
buf->lock = lock;
@@ -441,7 +441,7 @@ _evbuffer_decref_and_unlock(struct evbuffer *buffer)
EVBUFFER_UNLOCK(buffer, EVTHREAD_WRITE);
if (buffer->own_lock)
- EVTHREAD_FREE_LOCK(buffer->lock);
+ EVTHREAD_FREE_LOCK(buffer->lock, EVTHREAD_LOCKTYPE_RECURSIVE);
mm_free(buffer);
}
View
@@ -508,7 +508,8 @@ _bufferevent_decref_and_unlock(struct bufferevent *bufev)
BEV_UNLOCK(bufev);
if (bufev_private->own_lock)
- EVTHREAD_FREE_LOCK(bufev_private->lock);
+ EVTHREAD_FREE_LOCK(bufev_private->lock,
+ EVTHREAD_LOCKTYPE_RECURSIVE);
/* Free the actual allocated memory. */
mm_free(bufev - bufev->be_ops->mem_offset);
@@ -549,7 +550,7 @@ bufferevent_enable_locking(struct bufferevent *bufev, void *lock)
BEV_UPCAST(bufev)->lock = lock;
BEV_UPCAST(bufev)->own_lock = 0;
} else if (!lock) {
- EVTHREAD_ALLOC_LOCK(lock);
+ EVTHREAD_ALLOC_LOCK(lock, EVTHREAD_LOCKTYPE_RECURSIVE);
if (!lock)
return -1;
BEV_UPCAST(bufev)->lock = lock;
View
@@ -88,15 +88,13 @@ void event_deferred_cb_schedule(struct deferred_cb_queue *, struct deferred_cb *
#define LOCK_DEFERRED_QUEUE(q) \
do { \
if ((q)->lock) \
- _evthread_locking_fn(EVTHREAD_LOCK|EVTHREAD_WRITE, \
- (q)->lock); \
+ _evthread_lock_fns.lock(0, (q)->lock); \
} while (0)
#define UNLOCK_DEFERRED_QUEUE(q) \
do { \
if ((q)->lock) \
- _evthread_locking_fn(EVTHREAD_UNLOCK|EVTHREAD_WRITE, \
- (q)->lock); \
+ _evthread_lock_fns.unlock(0, (q)->lock); \
} while (0)
#endif
View
@@ -1688,7 +1688,7 @@ evdns_add_server_port_with_base(struct event_base *base, evutil_socket_t socket,
mm_free(port);
return NULL;
}
- EVTHREAD_ALLOC_LOCK(port->lock);
+ EVTHREAD_ALLOC_LOCK(port->lock, EVTHREAD_LOCKTYPE_RECURSIVE);
return port;
}
@@ -2082,7 +2082,7 @@ server_port_free(struct evdns_server_port *port)
port->socket = -1;
}
(void) event_del(&port->event);
- EVTHREAD_FREE_LOCK(port->lock);
+ EVTHREAD_FREE_LOCK(port->lock, EVTHREAD_LOCKTYPE_RECURSIVE);
mm_free(port);
}
@@ -3654,7 +3654,7 @@ evdns_base_new(struct event_base *event_base, int initialize_nameservers)
memset(base, 0, sizeof(struct evdns_base));
base->req_waiting_head = NULL;
- EVTHREAD_ALLOC_LOCK(base->lock);
+ EVTHREAD_ALLOC_LOCK(base->lock, EVTHREAD_LOCKTYPE_RECURSIVE);
EVDNS_LOCK(base);
/* Set max requests inflight and allocate req_heads. */
@@ -3773,7 +3773,7 @@ evdns_base_free_and_unlock(struct evdns_base *base, int fail_requests)
base->global_search_state = NULL;
}
EVDNS_UNLOCK(base);
- EVTHREAD_FREE_LOCK(base->lock);
+ EVTHREAD_FREE_LOCK(base->lock, EVTHREAD_LOCKTYPE_RECURSIVE);
mm_free(base);
}
View
@@ -352,9 +352,11 @@ event_base_new_with_config(struct event_config *cfg)
#ifndef _EVENT_DISABLE_THREAD_SUPPORT
if (!cfg || !(cfg->flags & EVENT_BASE_FLAG_NOLOCK)) {
int r;
- EVTHREAD_ALLOC_LOCK(base->th_base_lock);
+ EVTHREAD_ALLOC_LOCK(base->th_base_lock,
+ EVTHREAD_LOCKTYPE_RECURSIVE);
base->defer_queue.lock = base->th_base_lock;
- EVTHREAD_ALLOC_LOCK(base->current_event_lock);
+ EVTHREAD_ALLOC_LOCK(base->current_event_lock,
+ EVTHREAD_LOCKTYPE_RECURSIVE);
r = evthread_make_base_notifiable(base);
if (r<0) {
event_base_free(base);
@@ -475,8 +477,9 @@ event_base_free(struct event_base *base)
evmap_io_clear(&base->io);
evmap_signal_clear(&base->sigmap);
- EVTHREAD_FREE_LOCK(base->th_base_lock);
- EVTHREAD_FREE_LOCK(base->current_event_lock);
+ EVTHREAD_FREE_LOCK(base->th_base_lock, EVTHREAD_LOCKTYPE_RECURSIVE);
+ EVTHREAD_FREE_LOCK(base->current_event_lock,
+ EVTHREAD_LOCKTYPE_RECURSIVE);
mm_free(base);
}
@@ -2083,20 +2086,6 @@ event_set_mem_functions(void *(*malloc_fn)(size_t sz),
}
#endif
-#ifndef _EVENT_DISABLE_THREAD_SUPPORT
-/* support for threading */
-void (*_evthread_locking_fn)(int mode, void *lock) = NULL;
-unsigned long (*_evthread_id_fn)(void) = NULL;
-void *(*_evthread_lock_alloc_fn)(void) = NULL;
-void (*_evthread_lock_free_fn)(void *) = NULL;
-
-void
-evthread_set_locking_callback(void (*locking_fn)(int mode, void *lock))
-{
- _evthread_locking_fn = locking_fn;
-}
-#endif
-
#if defined(_EVENT_HAVE_EVENTFD) && defined(_EVENT_HAVE_SYS_EVENTFD_H)
static void
evthread_notify_drain_eventfd(int fd, short what, void *arg)
@@ -2120,14 +2109,6 @@ evthread_notify_drain_default(evutil_socket_t fd, short what, void *arg)
#endif
}
-#ifndef _EVENT_DISABLE_THREAD_SUPPORT
-void
-evthread_set_id_callback(unsigned long (*id_fn)(void))
-{
- _evthread_id_fn = id_fn;
-}
-#endif
-
int
evthread_make_base_notifiable(struct event_base *base)
{
@@ -2191,16 +2172,6 @@ evthread_make_base_notifiable(struct event_base *base)
return event_add(&base->th_notify, NULL);
}
-#ifndef _EVENT_DISABLE_THREAD_SUPPORT
-void
-evthread_set_lock_create_callbacks(void *(*alloc_fn)(void),
- void (*free_fn)(void *))
-{
- _evthread_lock_alloc_fn = alloc_fn;
- _evthread_lock_free_fn = free_fn;
-}
-#endif
-
void
event_base_dump_events(struct event_base *base, FILE *output)
{
View
@@ -38,10 +38,8 @@ struct event_base;
#ifndef _EVENT_DISABLE_THREAD_SUPPORT
/* Global function pointers to lock-related functions. NULL if locking isn't
enabled. */
-extern void (*_evthread_locking_fn)(int mode, void *lock);
+extern struct evthread_lock_callbacks _evthread_lock_fns;
extern unsigned long (*_evthread_id_fn)(void);
-extern void *(*_evthread_lock_alloc_fn)(void);
-extern void (*_evthread_lock_free_fn)(void *);
/** True iff the given event_base is set up to use locking */
#define EVBASE_USING_LOCKS(base) \
@@ -59,29 +57,30 @@ extern void (*_evthread_lock_free_fn)(void *);
/** Allocate a new lock, and store it in lockvar, a void*. Sets lockvar to
NULL if locking is not enabled. */
-#define EVTHREAD_ALLOC_LOCK(lockvar) \
- ((lockvar) = _evthread_lock_alloc_fn ? \
- _evthread_lock_alloc_fn() : NULL)
+#define EVTHREAD_ALLOC_LOCK(lockvar, locktype) \
+ ((lockvar) = _evthread_lock_fns.alloc ? \
+ _evthread_lock_fns.alloc(locktype) : NULL)
/** Free a given lock, if it is present and locking is enabled. */
-#define EVTHREAD_FREE_LOCK(lockvar) \
- do { \
- if (lockvar && _evthread_lock_free_fn) \
- _evthread_lock_free_fn(lockvar); \
+#define EVTHREAD_FREE_LOCK(lockvar, locktype) \
+ do { \
+ void *_lock_tmp_ = (lockvar); \
+ if (_lock_tmp_ && _evthread_lock_fns.free) \
+ _evthread_lock_fns.free(_lock_tmp_, (locktype)); \
} while (0)
/** Acquire a lock. */
-#define EVLOCK_LOCK(lock,mode) \
+#define EVLOCK_LOCK(lockvar,mode) \
do { \
- if (lock) \
- _evthread_locking_fn(EVTHREAD_LOCK|mode, lock); \
+ if (lockvar) \
+ _evthread_lock_fns.lock(mode, lockvar); \
} while (0)
/** Release a lock */
-#define EVLOCK_UNLOCK(lock,mode) \
+#define EVLOCK_UNLOCK(lockvar,mode) \
do { \
- if (lock) \
- _evthread_locking_fn(EVTHREAD_UNLOCK|mode, lock); \
+ if (lockvar) \
+ _evthread_lock_fns.unlock(mode, lockvar); \
} while (0)
/** Helper: put lockvar1 and lockvar2 into pointerwise ascending order. */
@@ -119,24 +118,22 @@ extern void (*_evthread_lock_free_fn)(void *);
/** Lock an event_base, if it is set up for locking. Acquires the lock
- in the base structure whose field is named 'lock'. */
-#define EVBASE_ACQUIRE_LOCK(base, mode, lock) do { \
+ in the base structure whose field is named 'lck'. */
+#define EVBASE_ACQUIRE_LOCK(base, mode, lockvar) do { \
if (EVBASE_USING_LOCKS(base)) \
- _evthread_locking_fn(EVTHREAD_LOCK | mode, \
- (base)->lock); \
+ _evthread_lock_fns.lock(mode, (base)->lockvar); \
} while (0)
/** Unlock an event_base, if it is set up for locking. */
-#define EVBASE_RELEASE_LOCK(base, mode, lock) do { \
+#define EVBASE_RELEASE_LOCK(base, mode, lockvar) do { \
if (EVBASE_USING_LOCKS(base)) \
- _evthread_locking_fn(EVTHREAD_UNLOCK | mode, \
- (base)->lock); \
+ _evthread_lock_fns.unlock(mode, (base)->lockvar); \
} while (0)
#else /* _EVENT_DISABLE_THREAD_SUPPORT */
#define EVTHREAD_GET_ID() 1
-#define EVTHREAD_ALLOC_LOCK(lockvar) _EVUTIL_NIL_STMT
-#define EVTHREAD_FREE_LOCK(lockvar) _EVUTIL_NIL_STMT
+#define EVTHREAD_ALLOC_LOCK(lockvar, locktype) _EVUTIL_NIL_STMT
+#define EVTHREAD_FREE_LOCK(lockvar, locktype) _EVUTIL_NIL_STMT
#define EVLOCK_LOCK(lockvar, mode) _EVUTIL_NIL_STMT
#define EVLOCK_UNLOCK(lockvar, mode) _EVUTIL_NIL_STMT
Oops, something went wrong. Retry.

0 comments on commit 347952f

Please sign in to comment.