From: moonlightsh <85744700@qq.com> Date: Wed, 17 Nov 2021 00:40:40 +0000 (+0800) Subject: Fix deadlock in case of evconnlistener_disable() in parallel with callback X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=12cedc8a4f3f01dd25d3eaf3b1d602d691233446;p=libevent Fix deadlock in case of evconnlistener_disable() in parallel with callback I've got an issue when stop evconnlistener not in the event_base_loop() thread. evconnlistener_disable() acquired lev->lock, if the same time, user callbacks is runing, the event thread released lock, after callback finished, it try to aquire the lock again, I think this makes conflict: Here is backtraces: thread 1: 0 __lll_lock_wait (futex=futex@entry=0x555555559a60, private=0) at lowlevellock.c:52 1 0x00007ffff7f2a131 in __GI___pthread_mutex_lock (mutex=0x555555559a60) at ../nptl/pthread_mutex_lock.c:115 2 0x00007ffff7f424c9 in evthread_posix_lock (mode=0, lock_=0x555555559a60) at evthread_pthread.c:79 3 0x00007ffff7f7dc12 in listener_read_cb (fd=7, what=2, p=0x5555555599a0) at listener.c:439 4 0x00007ffff7f6d758 in event_persist_closure (base=0x555555559370, ev=0x5555555599d8) at event.c:1645 5 0x00007ffff7f6da60 in event_process_active_single_queue (base=0x555555559370, activeq=0x5555555597e0, max_to_process=2147483647, endtime=0x0) at event.c:1704 6 0x00007ffff7f6e018 in event_process_active (base=0x555555559370) at event.c:1805 7 0x00007ffff7f6e92a in event_base_loop (base=0x555555559370, flags=0) at event.c:2047 8 0x0000555555555449 in main () at test_listen.c:67 thread 2: 0 futex_wait_cancelable (private=, expected=0, futex_word=0x555555559858) at ../sysdeps/nptl/futex-internal.h:183 1 __pthread_cond_wait_common (abstime=0x0, clockid=0, mutex=0x555555559800, cond=0x555555559830) at pthread_cond_wait.c:508 2 __pthread_cond_wait (cond=0x555555559830, mutex=0x555555559800) at pthread_cond_wait.c:638 3 0x00007ffff7f426f3 in evthread_posix_cond_wait (cond_=0x555555559830, lock_=0x555555559800, tv=0x0) at evthread_pthread.c:162 4 0x00007ffff7f70bc5 in event_del_nolock_ (ev=0x5555555599d8, blocking=2) at event.c:2934 5 0x00007ffff7f70748 in event_del_ (ev=0x5555555599d8, blocking=2) at event.c:2821 6 0x00007ffff7f707a1 in event_del (ev=0x5555555599d8) at event.c:2830 7 0x00007ffff7f7d76e in event_listener_disable (lev=0x5555555599a0) at listener.c:343 8 0x00007ffff7f7d6e5 in evconnlistener_disable (lev=0x5555555599a0) at listener.c:325 9 0x00005555555552c3 in disable_thread (arg=0x5555555599a0) at test_listen.c:27 10 0x00007ffff7f27609 in start_thread (arg=) at pthread_create.c:477 11 0x00007ffff7e4e293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 --- diff --git a/listener.c b/listener.c index bf55f7b5..125c7286 100644 --- a/listener.c +++ b/listener.c @@ -433,10 +433,8 @@ listener_read_cb(evutil_socket_t fd, short what, void *p) ++lev->refcnt; cb = lev->cb; user_data = lev->user_data; - UNLOCK(lev); cb(lev, new_fd, (struct sockaddr*)&ss, (int)socklen, user_data); - LOCK(lev); if (lev->refcnt == 1) { int freed = listener_decref_and_unlock(lev); EVUTIL_ASSERT(freed); @@ -458,9 +456,7 @@ listener_read_cb(evutil_socket_t fd, short what, void *p) ++lev->refcnt; errorcb = lev->errorcb; user_data = lev->user_data; - UNLOCK(lev); errorcb(lev, user_data); - LOCK(lev); listener_decref_and_unlock(lev); } else { event_sock_warn(fd, "Error from accept() call"); diff --git a/test/regress_listener.c b/test/regress_listener.c index 871da4c6..adf6697d 100644 --- a/test/regress_listener.c +++ b/test/regress_listener.c @@ -49,6 +49,11 @@ #include "event2/listener.h" #include "event2/event.h" #include "event2/util.h" +#ifndef EVENT__DISABLE_THREAD_SUPPORT +#include "event2/thread.h" +#include "regress_thread.h" +#endif + #include "regress.h" #include "tinytest.h" @@ -304,6 +309,127 @@ end: } #endif +#ifndef EVENT__DISABLE_THREAD_SUPPORT + +static THREAD_FN +disable_thread(void * arg) +{ + struct evconnlistener *lev = (struct evconnlistener *)arg; + evconnlistener_disable(lev); + return NULL; +} + +static void +acceptcb_for_thread_test(struct evconnlistener *listener, evutil_socket_t fd, + struct sockaddr *addr, int socklen, void *arg) +{ + THREAD_T *threadid_arg; + THREAD_T threadid; + struct timeval delay = { .tv_sec = 0, .tv_usec = 5000 }; + + threadid_arg = (THREAD_T *)arg; + + evutil_closesocket(fd); + + /* We need to run evconnlistener_disable() from disable_thread + * in parallel with processing this callback to trigger deadlock regression + */ + THREAD_START(threadid, disable_thread, listener); + evutil_usleep_(&delay); + + *threadid_arg = threadid; +} + +static void +regress_listener_disable_in_thread(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evconnlistener *listener = NULL; + struct sockaddr_in sin; + struct sockaddr_storage ss; + ev_socklen_t slen = sizeof(ss); + evutil_socket_t fd = EVUTIL_INVALID_SOCKET; + unsigned int flags = LEV_OPT_CLOSE_ON_FREE|LEV_OPT_REUSEABLE|LEV_OPT_THREADSAFE; + THREAD_T threadid; + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = htonl(0x7f000001); /* 127.0.0.1 */ + sin.sin_port = 0; /* "You pick!" */ + + + listener = evconnlistener_new_bind(base, acceptcb_for_thread_test, (void*)(&threadid), + flags, -1, (struct sockaddr *)&sin, sizeof(sin)); + + tt_assert(listener); + + tt_assert(getsockname(evconnlistener_get_fd(listener), + (struct sockaddr*)&ss, &slen) == 0); + + tt_assert(evutil_socket_connect_(&fd, (struct sockaddr*)&ss, slen) != -1); + + event_base_loop(base, EVLOOP_ONCE); + + THREAD_JOIN(threadid); +end: + if (listener) + evconnlistener_free(listener); +} + +static void +errorcb_for_thread_test(struct evconnlistener *listener, void *arg) +{ + THREAD_T *threadid_arg; + THREAD_T threadid; + struct timeval delay = { .tv_sec = 0, .tv_usec = 5000 }; + + threadid_arg = (THREAD_T *)arg; + + /* We need to run evconnlistener_disable() from disable_thread + * in parallel with processing this callback to trigger deadlock regression + */ + THREAD_START(threadid, disable_thread, listener); + evutil_usleep_(&delay); + + *threadid_arg = threadid; +} + +static void +acceptcb_for_thread_test_error(struct evconnlistener *listener, evutil_socket_t fd, + struct sockaddr *addr, int socklen, void *arg) +{ +} + +static void +regress_listener_disable_in_thread_error(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evconnlistener *listener = NULL; + unsigned int flags = LEV_OPT_CLOSE_ON_FREE|LEV_OPT_REUSEABLE|LEV_OPT_THREADSAFE; + THREAD_T threadid; + + /* send, so that pair[0] will look 'readable'*/ + tt_int_op(send(data->pair[1], "hello", 5, 0), >, 0); + + /* Start a listener with a bogus socket. */ + listener = evconnlistener_new(base, acceptcb_for_thread_test_error, (void*)&threadid, + flags, 0, + data->pair[0]); + tt_assert(listener); + + evconnlistener_set_error_cb(listener, errorcb_for_thread_test); + + event_base_loop(base, EVLOOP_ONCE); + + THREAD_JOIN(threadid); +end: + if (listener) + evconnlistener_free(listener); +} +#endif + struct testcase_t listener_testcases[] = { { "randport", regress_pick_a_port, TT_FORK|TT_NEED_BASE, @@ -332,6 +458,16 @@ struct testcase_t listener_testcases[] = { { "immediate_close", regress_listener_immediate_close, TT_FORK|TT_NEED_BASE, &basic_setup, NULL, }, +#ifndef EVENT__DISABLE_THREAD_SUPPORT + { "disable_in_thread", regress_listener_disable_in_thread, + TT_FORK|TT_NEED_BASE|TT_NEED_THREADS, + &basic_setup, NULL, }, + + { "disable_in_thread_error", regress_listener_disable_in_thread_error, + TT_FORK|TT_NEED_BASE|TT_NEED_THREADS|TT_NEED_SOCKETPAIR, + &basic_setup, NULL, }, +#endif + END_OF_TESTCASES, };