From 74ae43d6c88f18a795ee9e5aee6a7e211e6ba964 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Sun, 8 Nov 2009 21:04:18 +0000 Subject: [PATCH] reducing leaks, fixing shutdown bug, increasing timeouts --- src/core/test_core_api.c | 2 +- src/datastore/test_datastore_api_management.c | 2 +- src/fs/fs_file_information.c | 7 + src/fs/fs_tree.c | 11 +- src/fs/test_fs_download.c | 2 +- src/fs/test_fs_list_indexed.c | 2 +- src/fs/test_fs_publish.c | 2 +- src/fs/test_fs_search.c | 2 +- src/fs/test_fs_unindex.c | 2 +- src/include/gnunet_client_lib.h | 3 +- src/transport/test_transport_api.c | 2 +- src/util/client.c | 205 +++++++++--------- 12 files changed, 134 insertions(+), 108 deletions(-) diff --git a/src/core/test_core_api.c b/src/core/test_core_api.c index ebd4e8e99..6ad396415 100644 --- a/src/core/test_core_api.c +++ b/src/core/test_core_api.c @@ -42,7 +42,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) #define MTYPE 12345 diff --git a/src/datastore/test_datastore_api_management.c b/src/datastore/test_datastore_api_management.c index cad789a6f..a889f7c1c 100644 --- a/src/datastore/test_datastore_api_management.c +++ b/src/datastore/test_datastore_api_management.c @@ -33,7 +33,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) /** * Number of iterations to run; must be large enough diff --git a/src/fs/fs_file_information.c b/src/fs/fs_file_information.c index 2f8bbbdc9..c28635198 100644 --- a/src/fs/fs_file_information.c +++ b/src/fs/fs_file_information.c @@ -32,6 +32,7 @@ #include #include "gnunet_fs_service.h" #include "fs.h" +#include "fs_tree.h" /** @@ -849,6 +850,12 @@ GNUNET_FS_file_information_destroy (struct GNUNET_FS_FileInformation *fi, GNUNET_FS_uri_destroy (fi->keywords); GNUNET_CONTAINER_meta_data_destroy (fi->meta); GNUNET_free_non_null (fi->serialization); + if (fi->te != NULL) + { + GNUNET_FS_tree_encoder_finish (fi->te, + NULL, NULL); + fi->te = NULL; + } GNUNET_free (fi); } diff --git a/src/fs/fs_tree.c b/src/fs/fs_tree.c index 4d15d78f3..07f78eea7 100644 --- a/src/fs/fs_tree.c +++ b/src/fs/fs_tree.c @@ -385,8 +385,15 @@ void GNUNET_FS_tree_encoder_finish (struct GNUNET_FS_TreeEncoder * te, struct GNUNET_FS_Uri **uri, char **emsg) { - *uri = te->uri; - *emsg = te->emsg; + if (uri != NULL) + *uri = te->uri; + else + if (NULL != te->uri) + GNUNET_FS_uri_destroy (te->uri); + if (emsg != NULL) + *emsg = te->emsg; + else + GNUNET_free_non_null (te->emsg); GNUNET_free (te->chk_tree); GNUNET_free (te); } diff --git a/src/fs/test_fs_download.c b/src/fs/test_fs_download.c index f3ecfaba0..4b940e8dc 100644 --- a/src/fs/test_fs_download.c +++ b/src/fs/test_fs_download.c @@ -41,7 +41,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) /** * How long should our test-content live? diff --git a/src/fs/test_fs_list_indexed.c b/src/fs/test_fs_list_indexed.c index f9225e3ce..ba0906c81 100644 --- a/src/fs/test_fs_list_indexed.c +++ b/src/fs/test_fs_list_indexed.c @@ -45,7 +45,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) /** * How long should our test-content live? diff --git a/src/fs/test_fs_publish.c b/src/fs/test_fs_publish.c index a7bcd48d9..46a2dc3da 100644 --- a/src/fs/test_fs_publish.c +++ b/src/fs/test_fs_publish.c @@ -42,7 +42,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) /** * How long should our test-content live? diff --git a/src/fs/test_fs_search.c b/src/fs/test_fs_search.c index e0961c4a7..2ef323317 100644 --- a/src/fs/test_fs_search.c +++ b/src/fs/test_fs_search.c @@ -41,7 +41,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) /** * How long should our test-content live? diff --git a/src/fs/test_fs_unindex.c b/src/fs/test_fs_unindex.c index af9db7400..4c751c9a6 100644 --- a/src/fs/test_fs_unindex.c +++ b/src/fs/test_fs_unindex.c @@ -41,7 +41,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 15) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) /** * How long should our test-content live? diff --git a/src/include/gnunet_client_lib.h b/src/include/gnunet_client_lib.h index 3773a55d1..b2311e870 100644 --- a/src/include/gnunet_client_lib.h +++ b/src/include/gnunet_client_lib.h @@ -70,7 +70,8 @@ struct GNUNET_CLIENT_Connection *GNUNET_CLIENT_connect (struct * transmission request will also be cancelled UNLESS the callback for * the transmission request has already been called, in which case the * transmission is guaranteed to complete before the socket is fully - * destroyed. + * destroyed (unless, of course, there is an error with the server + * in which case the message may still be lost). * * @param sock handle to the service connection */ diff --git a/src/transport/test_transport_api.c b/src/transport/test_transport_api.c index 57ab52e4b..36264051d 100644 --- a/src/transport/test_transport_api.c +++ b/src/transport/test_transport_api.c @@ -40,7 +40,7 @@ /** * How long until we give up on transmitting the message? */ -#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 30) +#define TIMEOUT GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) #define MTYPE 12345 diff --git a/src/util/client.c b/src/util/client.c index 664b06271..a1ef36496 100644 --- a/src/util/client.c +++ b/src/util/client.c @@ -72,15 +72,15 @@ struct GNUNET_CLIENT_TransmitHandle struct GNUNET_CONNECTION_TransmitHandle *th; /** - * Timeout. + * If we are re-trying and are delaying to do so, + * handle to the scheduled task managing the delay. */ - struct GNUNET_TIME_Absolute timeout; + GNUNET_SCHEDULER_TaskIdentifier reconnect_task; /** - * If we are re-trying and are delaying to do so, - * handle to the scheduled task managing the delay. + * Timeout. */ - GNUNET_SCHEDULER_TaskIdentifier task; + struct GNUNET_TIME_Absolute timeout; /** * Number of bytes requested. @@ -104,6 +104,39 @@ struct GNUNET_CLIENT_TransmitHandle }; +/** + * Context for processing + * "GNUNET_CLIENT_transmit_and_get_response" requests. + */ +struct TransmitGetResponseContext +{ + /** + * Client handle. + */ + struct GNUNET_CLIENT_Connection *sock; + + /** + * Message to transmit; do not free, allocated + * right after this struct. + */ + const struct GNUNET_MessageHeader *hdr; + + /** + * Timeout to use. + */ + struct GNUNET_TIME_Absolute timeout; + + /** + * Function to call when done. + */ + GNUNET_CLIENT_MessageHandler rn; + + /** + * Closure for "rn". + */ + void *rn_cls; +}; + /** * Struct to refer to a GNUnet TCP connection. @@ -134,6 +167,12 @@ struct GNUNET_CLIENT_Connection */ char *service_name; + /** + * Context of a transmit_and_get_response operation, NULL + * if no such operation is pending. + */ + struct TransmitGetResponseContext *tag; + /** * Handler for current receiver task. */ @@ -155,6 +194,12 @@ struct GNUNET_CLIENT_Connection */ GNUNET_SCHEDULER_Task test_cb; + /** + * If we are re-trying and are delaying to do so, + * handle to the scheduled task managing the delay. + */ + GNUNET_SCHEDULER_TaskIdentifier receive_task; + /** * Closure for test_cb (NULL unless in service_test) */ @@ -187,9 +232,7 @@ struct GNUNET_CLIENT_Connection /** * Are we currently busy doing receive-processing? - * GNUNET_YES if so, GNUNET_NO if not, GNUNET_SYSERR - * if the handle should be destroyed as soon as the - * receive processing is done. + * GNUNET_YES if so, GNUNET_NO if not. */ int in_receive; @@ -268,24 +311,6 @@ GNUNET_CLIENT_connect (struct GNUNET_SCHEDULER_Handle *sched, } -/** - * Receiver task has completed, free rest of client - * data structures. - */ -static void -finish_cleanup (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc) -{ - struct GNUNET_CLIENT_Connection *sock = cls; - - if (sock->th != NULL) - GNUNET_CLIENT_notify_transmit_ready_cancel (sock->th); - GNUNET_array_grow (sock->received_buf, sock->received_size, 0); - GNUNET_free (sock->service_name); - GNUNET_CONFIGURATION_destroy (sock->cfg); - GNUNET_free (sock); -} - - /** * Destroy connection with the service. This will automatically * cancel any pending "receive" request (however, the handler will @@ -293,7 +318,8 @@ finish_cleanup (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc) * transmission request will also be cancelled UNLESS the callback for * the transmission request has already been called, in which case the * transmission is guaranteed to complete before the socket is fully - * destroyed. + * destroyed (unless, of course, there is an error with the server + * in which case the message may still be lost). * * @param sock handle to the service connection */ @@ -301,15 +327,31 @@ void GNUNET_CLIENT_disconnect (struct GNUNET_CLIENT_Connection *sock) { GNUNET_assert (sock->sock != NULL); + if (sock->in_receive == GNUNET_YES) + { + GNUNET_CONNECTION_receive_cancel (sock->sock); + sock->in_receive = GNUNET_NO; + } GNUNET_CONNECTION_destroy (sock->sock); sock->sock = NULL; + if (sock->tag != NULL) + { + GNUNET_free (sock->tag); + sock->tag = NULL; + } sock->receiver_handler = NULL; - if (sock->in_receive == GNUNET_YES) - sock->in_receive = GNUNET_SYSERR; - else - GNUNET_SCHEDULER_add_after (sock->sched, - GNUNET_SCHEDULER_NO_TASK, - &finish_cleanup, sock); + if (sock->th != NULL) + GNUNET_CLIENT_notify_transmit_ready_cancel (sock->th); + if (sock->receive_task != GNUNET_SCHEDULER_NO_TASK) + { + GNUNET_SCHEDULER_cancel (sock->sched, + sock->receive_task); + sock->receive_task = GNUNET_SCHEDULER_NO_TASK; + } + GNUNET_array_grow (sock->received_buf, sock->received_size, 0); + GNUNET_free (sock->service_name); + GNUNET_CONFIGURATION_destroy (sock->cfg); + GNUNET_free (sock); } @@ -349,10 +391,6 @@ receive_helper (void *cls, struct GNUNET_TIME_Relative remaining; GNUNET_assert (conn->msg_complete == GNUNET_NO); - if (GNUNET_SYSERR == conn->in_receive) - GNUNET_SCHEDULER_add_after (conn->sched, - GNUNET_SCHEDULER_NO_TASK, - &finish_cleanup, conn); conn->in_receive = GNUNET_NO; if ((available == 0) || (conn->sock == NULL) || (errCode != 0)) { @@ -393,24 +431,23 @@ receive_helper (void *cls, /** * Continuation to call the receive callback. + * + * @param cls our handle to the client connection + * @param tc scheduler context */ static void -receive_task (void *scls, const struct GNUNET_SCHEDULER_TaskContext *tc) +receive_task (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc) { - struct GNUNET_CLIENT_Connection *sock = scls; + struct GNUNET_CLIENT_Connection *sock = cls; GNUNET_CLIENT_MessageHandler handler = sock->receiver_handler; const struct GNUNET_MessageHeader *cmsg = (const struct GNUNET_MessageHeader *) sock->received_buf; - void *cls = sock->receiver_handler_cls; + void *handler_cls = sock->receiver_handler_cls; uint16_t msize = ntohs (cmsg->size); char mbuf[msize]; struct GNUNET_MessageHeader *msg = (struct GNUNET_MessageHeader *) mbuf; - if (GNUNET_SYSERR == sock->in_receive) - GNUNET_SCHEDULER_add_after (sock->sched, - GNUNET_SCHEDULER_NO_TASK, - &finish_cleanup, sock); - sock->in_receive = GNUNET_NO; + sock->receive_task = GNUNET_SCHEDULER_NO_TASK; GNUNET_assert (GNUNET_YES == sock->msg_complete); GNUNET_assert (sock->received_pos >= msize); memcpy (msg, cmsg, msize); @@ -421,7 +458,7 @@ receive_task (void *scls, const struct GNUNET_SCHEDULER_TaskContext *tc) sock->receiver_handler = NULL; check_complete (sock); if (handler != NULL) - handler (cls, msg); + handler (handler_cls, msg); } @@ -448,15 +485,19 @@ GNUNET_CLIENT_receive (struct GNUNET_CLIENT_Connection *sock, sock->receiver_handler = handler; sock->receiver_handler_cls = handler_cls; sock->receive_timeout = GNUNET_TIME_relative_to_absolute (timeout); - sock->in_receive = GNUNET_YES; if (GNUNET_YES == sock->msg_complete) - GNUNET_SCHEDULER_add_after (sock->sched, - GNUNET_SCHEDULER_NO_TASK, - &receive_task, sock); + { + sock->receive_task = GNUNET_SCHEDULER_add_after (sock->sched, + GNUNET_SCHEDULER_NO_TASK, + &receive_task, sock); + } else - GNUNET_CONNECTION_receive (sock->sock, - GNUNET_SERVER_MAX_MESSAGE_SIZE, - timeout, &receive_helper, sock); + { + sock->in_receive = GNUNET_YES; + GNUNET_CONNECTION_receive (sock->sock, + GNUNET_SERVER_MAX_MESSAGE_SIZE, + timeout, &receive_helper, sock); + } } @@ -657,7 +698,7 @@ client_delayed_retry (void *cls, { struct GNUNET_CLIENT_TransmitHandle *th = cls; - th->task = GNUNET_SCHEDULER_NO_TASK; + th->reconnect_task = GNUNET_SCHEDULER_NO_TASK; if (0 != (tc->reason & GNUNET_SCHEDULER_REASON_SHUTDOWN)) { #if DEBUG_CLIENT @@ -733,9 +774,9 @@ client_notify (void *cls, size_t size, void *buf) MAX_ATTEMPTS - th->attempts_left, (unsigned long long) delay.value); #endif - th->task = GNUNET_SCHEDULER_add_delayed (th->sock->sched, - delay, - &client_delayed_retry, th); + th->reconnect_task = GNUNET_SCHEDULER_add_delayed (th->sock->sched, + delay, + &client_delayed_retry, th); th->sock->th = th; return 0; } @@ -809,10 +850,11 @@ void GNUNET_CLIENT_notify_transmit_ready_cancel (struct GNUNET_CLIENT_TransmitHandle *th) { - if (th->task != GNUNET_SCHEDULER_NO_TASK) + if (th->reconnect_task != GNUNET_SCHEDULER_NO_TASK) { GNUNET_break (NULL == th->th); - GNUNET_SCHEDULER_cancel (th->sock->sched, th->task); + GNUNET_SCHEDULER_cancel (th->sock->sched, th->reconnect_task); + th->reconnect_task = GNUNET_SCHEDULER_NO_TASK; } else { @@ -824,47 +866,13 @@ GNUNET_CLIENT_notify_transmit_ready_cancel (struct } -/** - * Context for processing - * "GNUNET_CLIENT_transmit_and_get_response" requests. - */ -struct TARCtx -{ - /** - * Client handle. - */ - struct GNUNET_CLIENT_Connection *sock; - - /** - * Message to transmit; do not free, allocated - * right after this struct. - */ - const struct GNUNET_MessageHeader *hdr; - - /** - * Timeout to use. - */ - struct GNUNET_TIME_Absolute timeout; - - /** - * Function to call when done. - */ - GNUNET_CLIENT_MessageHandler rn; - - /** - * Closure for "rn". - */ - void *rn_cls; -}; - - /** * Function called to notify a client about the socket * begin ready to queue the message. "buf" will be * NULL and "size" zero if the socket was closed for * writing in the meantime. * - * @param cls closure of type "struct TARCtx*" + * @param cls closure of type "struct TransmitGetResponseContext*" * @param size number of bytes available in buf * @param buf where the callee should write the message * @return number of bytes written to buf @@ -872,9 +880,10 @@ struct TARCtx static size_t transmit_for_response (void *cls, size_t size, void *buf) { - struct TARCtx *tc = cls; + struct TransmitGetResponseContext *tc = cls; uint16_t msize; + tc->sock->tag = NULL; msize = ntohs (tc->hdr->size); if (NULL == buf) { @@ -924,13 +933,14 @@ GNUNET_CLIENT_transmit_and_get_response (struct GNUNET_CLIENT_Connection GNUNET_CLIENT_MessageHandler rn, void *rn_cls) { - struct TARCtx *tc; + struct TransmitGetResponseContext *tc; uint16_t msize; if (NULL != sock->th) return GNUNET_SYSERR; + GNUNET_assert (sock->tag == NULL); msize = ntohs (hdr->size); - tc = GNUNET_malloc (sizeof (struct TARCtx) + msize); + tc = GNUNET_malloc (sizeof (struct TransmitGetResponseContext) + msize); tc->sock = sock; tc->hdr = (const struct GNUNET_MessageHeader *) &tc[1]; memcpy (&tc[1], hdr, msize); @@ -948,6 +958,7 @@ GNUNET_CLIENT_transmit_and_get_response (struct GNUNET_CLIENT_Connection GNUNET_free (tc); return GNUNET_SYSERR; } + sock->tag = tc; return GNUNET_OK; } -- 2.25.1