From 83da6bcc7751ee11905e3f8f3adcfe33d2b926bc Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Tue, 9 Jun 2015 09:55:11 +0000 Subject: -avoid concurrent modification trouble when handing peer disconnect --- configure.ac | 115 +++++++++++++++------------- po/POTFILES.in | 2 + src/cadet/gnunet-service-cadet_connection.c | 11 +-- src/cadet/gnunet-service-cadet_peer.c | 93 ++++++++++++++-------- src/cadet/gnunet-service-cadet_peer.h | 8 +- 5 files changed, 136 insertions(+), 93 deletions(-) diff --git a/configure.ac b/configure.ac index 3072684fc..67633b107 100644 --- a/configure.ac +++ b/configure.ac @@ -491,26 +491,6 @@ else fi -# libidn -AC_MSG_CHECKING([if Libidn can be used]) -AC_ARG_WITH(libidn, AC_HELP_STRING([--with-libidn=[DIR]], - [Support IDN (needs GNU Libidn)]), -libidn=$withval, libidn=yes) -if test "$libidn" != "no"; then - if test "$libidn" != "yes"; then - LDFLAGS="${LDFLAGS} -L$libidn/lib" - CPPFLAGS="${CPPFLAGS} -I$libidn/include" - fi -fi -libidn=no -AC_CHECK_HEADER(idna.h, - AC_CHECK_LIB(idn, stringprep_check_version, - [libidn=yes LIBS="${LIBS} -lidn"], []), []) -if test "$libidn" != "yes"; then - AC_MSG_FAILURE([GNUnet requires libidn. -libidn-1.13 should be sufficient, newer versions work too.]) -fi -AC_MSG_RESULT($libidn) # restore LIBS LIBS=$SAVE_LIBS @@ -639,7 +619,69 @@ fi # restore LIBS LIBS=$SAVE_LIBS + +# libidn +AC_MSG_CHECKING([if Libidn can be used]) +AC_ARG_WITH(libidn, AC_HELP_STRING([--with-libidn=[DIR]], + [Support IDN (needs GNU Libidn)]), +libidn=$withval, libidn=yes) +if test "$libidn" != "no"; then + if test "$libidn" != "yes"; then + LDFLAGS="${LDFLAGS} -L$libidn/lib" + CPPFLAGS="${CPPFLAGS} -I$libidn/include" + fi fi +libidn=no +AC_CHECK_HEADER(idna.h, + AC_CHECK_LIB(idn, stringprep_check_version, + [libidn=yes LIBS="${LIBS} -lidn"], []), []) +if test "$libidn" != "yes"; then + AC_MSG_FAILURE([GNUnet requires libidn. +libidn-1.13 should be sufficient, newer versions work too.]) +fi +AC_MSG_RESULT($libidn) + + +# test for zlib +SAVE_LDFLAGS=$LDFLAGS +SAVE_CPPFLAGS=$CPPFLAGS +AC_ARG_WITH(zlib, + [ --with-zlib[[=DIR]] use libz in DIR], + [AS_IF([test "$withval" = "no"], + [AC_MSG_ERROR([GNUnet requires zlib])], + [test "$withval" != "yes"], + [ + Z_DIR=$withval + CPPFLAGS="${CPPFLAGS} -I$withval/include" + LDFLAGS="${LDFLAGS} -L$withval/lib" + ]) + ]) +AC_CHECK_HEADER(zlib.h, + [], + [AC_MSG_ERROR([GNUnet requires zlib])]) +AC_CHECK_LIB(z, compress2, + [ + AC_DEFINE([HAVE_ZLIB], [], [Have compression library]) + if test "x${Z_DIR}" != "x"; then + Z_CFLAGS="-I${Z_DIR}/include" + Z_LIBS="-L${Z_DIR}/lib -lz" + else + Z_LIBS="-lz" + fi], + [AC_MSG_ERROR([GNUnet requires zlib])]) +AC_SUBST(Z_CFLAGS) +AC_SUBST(Z_LIBS) + + + +# restore LIBS +LIBS=$SAVE_LIBS + + +fi + +# check for iconv +AM_ICONV # test for libunistring gl_LIBUNISTRING @@ -722,36 +764,6 @@ if test "$found_postgresql" = "yes"; then fi AM_CONDITIONAL(HAVE_POSTGRESQL, test x$postgres = xtrue) -# test for zlib -SAVE_LDFLAGS=$LDFLAGS -SAVE_CPPFLAGS=$CPPFLAGS -AC_ARG_WITH(zlib, - [ --with-zlib[[=DIR]] use libz in DIR], - [AS_IF([test "$withval" = "no"], - [AC_MSG_ERROR([GNUnet requires zlib])], - [test "$withval" != "yes"], - [ - Z_DIR=$withval - CPPFLAGS="${CPPFLAGS} -I$withval/include" - LDFLAGS="${LDFLAGS} -L$withval/lib" - ]) - ]) -AC_CHECK_HEADER(zlib.h, - [], - [AC_MSG_ERROR([GNUnet requires zlib])]) -AC_CHECK_LIB(z, compress2, - [ - AC_DEFINE([HAVE_ZLIB], [], [Have compression library]) - if test "x${Z_DIR}" != "x"; then - Z_CFLAGS="-I${Z_DIR}/include" - Z_LIBS="-L${Z_DIR}/lib -lz" - else - Z_LIBS="-lz" - fi], - [AC_MSG_ERROR([GNUnet requires zlib])]) -AC_SUBST(Z_CFLAGS) -AC_SUBST(Z_LIBS) - LDFLAGS=$SAVE_LDFLAGS CPPFLAGS=$SAVE_CPPFLAGS @@ -930,9 +942,6 @@ AM_CONDITIONAL([HAVE_PYTHON], [test "$PYTHON" != :]) AM_GNU_GETTEXT([external]) AM_GNU_GETTEXT_VERSION([0.19.3]) -# check for iconv -AM_ICONV - # Checks for standard typedefs, structures, and compiler characteristics. AC_TYPE_PID_T AC_TYPE_SIZE_T diff --git a/po/POTFILES.in b/po/POTFILES.in index 34bdaedd8..dd166adcc 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -274,6 +274,7 @@ src/rps/gnunet-rps.c src/rps/gnunet-service-rps.c src/rps/gnunet-service-rps_sampler.c src/rps/rps_api.c +src/rps/rps-test_util.c src/scalarproduct/gnunet-scalarproduct.c src/scalarproduct/gnunet-service-scalarproduct_alice.c src/scalarproduct/gnunet-service-scalarproduct_bob.c @@ -457,6 +458,7 @@ src/include/gnunet_common.h src/include/gnunet_mq_lib.h src/include/gnunet_postgres_lib.h src/include/gnunet_time_lib.h +src/rps/rps-test_util.h src/scalarproduct/scalarproduct.h src/testbed/testbed_api.h src/testbed/testbed_api_operations.h diff --git a/src/cadet/gnunet-service-cadet_connection.c b/src/cadet/gnunet-service-cadet_connection.c index dd4577447..47b4f3955 100644 --- a/src/cadet/gnunet-service-cadet_connection.c +++ b/src/cadet/gnunet-service-cadet_connection.c @@ -1547,8 +1547,8 @@ register_neighbors (struct CadetConnection *c) GCP_is_neighbor (c->next_peer)); return GNUNET_SYSERR; } - GCP_add_connection (c->next_peer, c); - GCP_add_connection (c->prev_peer, c); + GCP_add_connection (c->next_peer, c, GNUNET_NO); + GCP_add_connection (c->prev_peer, c, GNUNET_YES); return GNUNET_OK; } @@ -1566,10 +1566,10 @@ unregister_neighbors (struct CadetConnection *c) peer = get_next_hop (c); GNUNET_assert (c->next_peer == peer); - GCP_remove_connection (peer, c); + GCP_remove_connection (peer, c, GNUNET_NO); peer = get_prev_hop (c); GNUNET_assert (c->prev_peer == peer); - GCP_remove_connection (peer, c); + GCP_remove_connection (peer, c, GNUNET_YES); } @@ -1583,7 +1583,8 @@ unregister_neighbors (struct CadetConnection *c) * @param peer Peer. */ static void -add_to_peer (struct CadetConnection *c, struct CadetPeer *peer) +add_to_peer (struct CadetConnection *c, + struct CadetPeer *peer) { GCP_add_tunnel (peer); c->t = GCP_get_tunnel (peer); diff --git a/src/cadet/gnunet-service-cadet_peer.c b/src/cadet/gnunet-service-cadet_peer.c index 705165c51..4abcd1a0b 100644 --- a/src/cadet/gnunet-service-cadet_peer.c +++ b/src/cadet/gnunet-service-cadet_peer.c @@ -156,9 +156,17 @@ struct CadetPeer struct CadetTunnel *tunnel; /** - * Connections that go through this peer, indexed by tid; + * Connections that go through this peer where we + * are the predecessor; indexed by tid; do NOT + * try to combine with @e connections_succ (#3794). */ - struct GNUNET_CONTAINER_MultiHashMap *connections; + struct GNUNET_CONTAINER_MultiHashMap *connections_pred; + + /** + * Connections that go through this peer where we are + * the successor; indexed by tid; + */ + struct GNUNET_CONTAINER_MultiHashMap *connections_succ; /** * Handle for queued transmissions @@ -314,10 +322,11 @@ GCP_debug (const struct CadetPeer *p, enum GNUNET_ErrorType level) LOG2 (level, "PPP core transmit handle %p\n", p->core_transmit); LOG2 (level, "PPP DHT GET handle %p\n", p->search_h); - if (NULL != p->connections) - conns = GNUNET_CONTAINER_multihashmap_size (p->connections); - else - conns = 0; + conns = 0; + if (NULL != p->connections_pred) + conns = GNUNET_CONTAINER_multihashmap_size (p->connections_pred); + if (NULL != p->connections_succ) + conns += GNUNET_CONTAINER_multihashmap_size (p->connections_succ); LOG2 (level, "PPP # connections over link to peer: %u\n", conns); queue_debug (p, level); LOG2 (level, "PPP DEBUG END\n"); @@ -428,8 +437,10 @@ core_connect (void *cls, "# peers", 1, GNUNET_NO); - GNUNET_assert (NULL == mp->connections); - mp->connections = GNUNET_CONTAINER_multihashmap_create (32, GNUNET_YES); + GNUNET_assert (NULL == mp->connections_pred); + GNUNET_assert (NULL == mp->connections_succ); + mp->connections_pred = GNUNET_CONTAINER_multihashmap_create (16, GNUNET_YES); + mp->connections_succ = GNUNET_CONTAINER_multihashmap_create (16, GNUNET_YES); if ( (NULL != GCP_get_tunnel (mp)) && (0 > GNUNET_CRYPTO_cmp_peer_identity (&my_full_id, peer)) ) @@ -468,11 +479,16 @@ core_disconnect (void *cls, "DISCONNECTED %s <= %s\n", own_id, GNUNET_i2s (peer)); direct_path = pop_direct_path (p); - GNUNET_CONTAINER_multihashmap_iterate (p->connections, + GNUNET_CONTAINER_multihashmap_iterate (p->connections_succ, + ¬ify_broken, + p); + GNUNET_CONTAINER_multihashmap_iterate (p->connections_pred, ¬ify_broken, p); - GNUNET_CONTAINER_multihashmap_destroy (p->connections); - p->connections = NULL; + GNUNET_CONTAINER_multihashmap_destroy (p->connections_succ); + p->connections_succ = NULL; + GNUNET_CONTAINER_multihashmap_destroy (p->connections_pred); + p->connections_pred = NULL; if (NULL != p->core_transmit) { GNUNET_CORE_notify_transmit_ready_cancel (p->core_transmit); @@ -1366,7 +1382,8 @@ GCP_queue_add (struct CadetPeer *peer, void *cls, uint16_t type, if (error_level == GNUNET_ERROR_TYPE_ERROR) GNUNET_assert (0); - if (NULL == peer->connections) + if ( (NULL == peer->connections_pred) || + (NULL == peer->connections_succ) ) { /* We are not connected to this peer, ignore request. */ LOG (GNUNET_ERROR_TYPE_WARNING, "%s not a neighbor\n", GCP_2s (peer)); @@ -1849,11 +1866,11 @@ GCP_connect (struct CadetPeer *peer) * not yet known to be connected. * * This happens quite often during testing when running cadet - * under valgrind: core connect notifications come very late and the - * DHT result has already come and created a valid path. - * In this case, the peer->connections hashmap will be NULL and - * tunnel_use_path will not be able to create a connection from that - * path. + * under valgrind: core connect notifications come very late + * and the DHT result has already come and created a valid + * path. In this case, the peer->connections_{pred,succ} + * hashmaps will be NULL and tunnel_use_path will not be able + * to create a connection from that path. * * Re-running the DHT GET should give core time to callback. * @@ -1902,7 +1919,8 @@ GCP_is_neighbor (const struct CadetPeer *peer) { struct CadetPeerPath *path; - if (NULL == peer->connections) + if ( (NULL == peer->connections_pred) || + (NULL == peer->connections_succ) ) return GNUNET_NO; for (path = peer->path_head; NULL != path; path = path->next) @@ -1942,10 +1960,12 @@ GCP_add_tunnel (struct CadetPeer *peer) * * @param peer Peer to add connection to. * @param c Connection to add. + * @param pred #GNUNET_YES if we are predecessor, #GNUNET_NO if we are successor */ void GCP_add_connection (struct CadetPeer *peer, - struct CadetConnection *c) + struct CadetConnection *c, + int pred) { LOG (GNUNET_ERROR_TYPE_DEBUG, "adding connection %s\n", @@ -1953,19 +1973,20 @@ GCP_add_connection (struct CadetPeer *peer, LOG (GNUNET_ERROR_TYPE_DEBUG, "to peer %s\n", GCP_2s (peer)); - GNUNET_assert (NULL != peer->connections); - LOG (GNUNET_ERROR_TYPE_DEBUG, - "peer %s has %u connections.\n", - GCP_2s (peer), - GNUNET_CONTAINER_multihashmap_size (peer->connections)); + GNUNET_assert (NULL != peer->connections_pred); + GNUNET_assert (NULL != peer->connections_succ); GNUNET_assert (GNUNET_OK == - GNUNET_CONTAINER_multihashmap_put (peer->connections, + GNUNET_CONTAINER_multihashmap_put ((GNUNET_YES == pred) + ? peer->connections_pred + : peer->connections_succ, GCC_get_h (c), c, GNUNET_CONTAINER_MULTIHASHMAPOPTION_MULTIPLE)); LOG (GNUNET_ERROR_TYPE_DEBUG, - " now has %u connections.\n", - GNUNET_CONTAINER_multihashmap_size (peer->connections)); + "Peer %s is now predecessor on %u connections and successor on %u connections.\n", + GCP_2s (peer), + GNUNET_CONTAINER_multihashmap_size (peer->connections_pred), + GNUNET_CONTAINER_multihashmap_size (peer->connections_succ)); } @@ -2159,10 +2180,12 @@ GCP_remove_path (struct CadetPeer *peer, struct CadetPeerPath *path) * * @param peer Peer to remove connection from. * @param c Connection to remove. + * @param pred #GNUNET_YES if we were predecessor, #GNUNET_NO if we were successor */ void GCP_remove_connection (struct CadetPeer *peer, - const struct CadetConnection *c) + const struct CadetConnection *c, + int pred) { LOG (GNUNET_ERROR_TYPE_DEBUG, "removing connection %s\n", @@ -2170,19 +2193,23 @@ GCP_remove_connection (struct CadetPeer *peer, LOG (GNUNET_ERROR_TYPE_DEBUG, "from peer %s\n", GCP_2s (peer)); - if ( (NULL == peer) || - (NULL == peer->connections) ) + (NULL == peer->connections_pred) || + (NULL == peer->connections_succ) ) return; - (void) GNUNET_CONTAINER_multihashmap_remove (peer->connections, + (void) GNUNET_CONTAINER_multihashmap_remove ((GNUNET_YES == pred) + ? peer->connections_pred + : peer->connections_succ, GCC_get_h (c), c); LOG (GNUNET_ERROR_TYPE_DEBUG, - "peer %s ok, has %u connections left.\n", + "Peer %s remains predecessor for %u and successor for %u connections.\n", GCP_2s (peer), - GNUNET_CONTAINER_multihashmap_size (peer->connections)); + GNUNET_CONTAINER_multihashmap_size (peer->connections_pred), + GNUNET_CONTAINER_multihashmap_size (peer->connections_succ)); } + /** * Start the DHT search for new paths towards the peer: we don't have * enough good connections. diff --git a/src/cadet/gnunet-service-cadet_peer.h b/src/cadet/gnunet-service-cadet_peer.h index af937007a..eae7764c3 100644 --- a/src/cadet/gnunet-service-cadet_peer.h +++ b/src/cadet/gnunet-service-cadet_peer.h @@ -247,10 +247,12 @@ GCP_add_tunnel (struct CadetPeer *peer); * * @param peer Peer to add connection to. * @param c Connection to add. + * @param pred #GNUNET_YES if we are predecessor, #GNUNET_NO if we are successor */ void GCP_add_connection (struct CadetPeer *peer, - struct CadetConnection *c); + struct CadetConnection *c, + int pred); /** @@ -316,10 +318,12 @@ GCP_remove_path (struct CadetPeer *peer, * * @param peer Peer to remove connection from. * @param c Connection to remove. + * @param pred #GNUNET_YES if we were predecessor, #GNUNET_NO if we were successor */ void GCP_remove_connection (struct CadetPeer *peer, - const struct CadetConnection *c); + const struct CadetConnection *c, + int pred); /** -- cgit v1.2.3