[PATCH] fix unregister/callback race in conn.c
"Alexander Malysh" <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi All, attached patch fixes race condition in unregister/callback usage/destroy of callback data. The race is as follow, spotted with help of http.c: 1 Thread 2 Thread 1) http:conn_pool_get 1) http:check_pool_conn 2) conn = XYZ 3) conn_unregister in conn_unregister destroy callbackdata 2) dict_get(conn_pool, key) -> crash because connback data already destroyed @Vincent: you reported similar crash. This patch should fix it. Votes please? -- Thanks, Alex
conn-unreg-callb-race.diff
(application/octet-stream, 2 KB)
=== gwlib/http.c
==================================================================
--- gwlib/http.c (revision 241)
+++ gwlib/http.c (local)
@@ -848,7 +848,7 @@
List *list;
mutex_lock(conn_pool_lock);
list = dict_get(conn_pool, key);
- if (list != NULL && gwlist_delete_equal(list, conn) > 0) {
+ if (gwlist_delete_equal(list, conn) > 0) {
/*
* ok, connection was still within pool. So it's
* safe to destroy this connection.
=== gwlib/conn.c
==================================================================
--- gwlib/conn.c (revision 241)
+++ gwlib/conn.c (local)
@@ -818,7 +818,9 @@
void conn_unregister(Connection *conn)
{
FDSet *set = NULL;
- int fd;
+ int fd = -1;
+ void *data = NULL;
+ conn_callback_data_destroyer_t *destroyer = NULL;
gw_assert(conn != NULL);
@@ -834,10 +836,14 @@
fd = conn->fd;
conn->registered = NULL;
conn->callback = NULL;
- /* call data destroyer */
- if (conn->callback_data != NULL && conn->callback_data_destroyer != NULL)
- conn->callback_data_destroyer(conn->callback_data);
+ /*
+ * remember and don't destroy data and data_destroyer because we
+ * may be in callback right now. So destroy only after fdset_unregister
+ * call which guarantee us we are not in callback anymore.
+ */
+ data = conn->callback_data;
conn->callback_data = NULL;
+ destroyer = conn->callback_data_destroyer;
conn->callback_data_destroyer = NULL;
conn->listening_pollin = 0;
conn->listening_pollout = 0;
@@ -845,10 +851,14 @@
unlock_in(conn);
unlock_out(conn);
-
+
/* now unregister from FDSet */
if (set != NULL)
fdset_unregister(set, fd);
+
+ /* ok we are not in callback anymore, destroy data if any */
+ if (data != NULL && destroyer != NULL)
+ destroyer(data);
}
int conn_wait(Connection *conn, double seconds)