From 76263deaff29f333c45a980e6093c0c4b4279ffb Mon Sep 17 00:00:00 2001 From: Simon Morlat Date: Tue, 22 Sep 2015 22:43:31 +0200 Subject: [PATCH] fix crash in linphone_core_destroy() when releasing the chat rooms while belle_sip_provider_t no longer exists, and fix many memory leaks --- coreapi/chat.c | 15 ++++++++++++--- coreapi/chat_file_transfer.c | 28 +++++++++++++++------------- coreapi/linphonecore.c | 6 +++--- coreapi/private.h | 1 + tester/liblinphone_tester.c | 5 ++++- tester/liblinphone_tester.h | 6 ++++++ tester/tester.c | 25 +++++++++++++++++-------- 7 files changed, 58 insertions(+), 28 deletions(-) diff --git a/coreapi/chat.c b/coreapi/chat.c index 9348f5447..0b11c6db5 100644 --- a/coreapi/chat.c +++ b/coreapi/chat.c @@ -383,6 +383,13 @@ void _linphone_chat_room_send_message(LinphoneChatRoom *cr, LinphoneChatMessage } msg->dir = LinphoneChatMessageOutgoing; + if (msg->from){ + /* + * BUG + * the file transfer message constructor sets the from, but doesn't do it as well as here. + */ + linphone_address_destroy(msg->from); + } msg->from = linphone_address_new(identity); msg->storage_id = linphone_chat_message_store(msg); @@ -466,9 +473,11 @@ void linphone_core_message_received(LinphoneCore *lc, SalOp *op, const SalMessag } if (!xmlStrcmp(cur->name, (const xmlChar *)"file-name")) { + xmlChar *filename = xmlNodeListGetString(xmlMessageBody, cur->xmlChildrenNode, 1); linphone_content_set_name( msg->file_transfer_information, - (const char *)xmlNodeListGetString(xmlMessageBody, cur->xmlChildrenNode, 1)); + (char *)filename); + xmlFree(filename); } if (!xmlStrcmp(cur->name, (const xmlChar *)"content-type")) { xmlChar *contentType = xmlNodeListGetString(xmlMessageBody, cur->xmlChildrenNode, 1); @@ -730,11 +739,11 @@ LinphoneChatMessage *linphone_chat_room_create_message_2(LinphoneChatRoom *cr, c if (is_incoming) { msg->dir = LinphoneChatMessageIncoming; linphone_chat_message_set_from(msg, linphone_chat_room_get_peer_address(cr)); - linphone_chat_message_set_to(msg, linphone_address_new(linphone_core_get_identity(lc))); + msg->to = linphone_address_new(linphone_core_get_identity(lc)); /*direct assignment*/ } else { msg->dir = LinphoneChatMessageOutgoing; linphone_chat_message_set_to(msg, linphone_chat_room_get_peer_address(cr)); - linphone_chat_message_set_from(msg, linphone_address_new(linphone_core_get_identity(lc))); + msg->from = linphone_address_new(linphone_core_get_identity(lc));/*direct assignment*/ } return msg; } diff --git a/coreapi/chat_file_transfer.c b/coreapi/chat_file_transfer.c index a70536a86..cd294cc90 100644 --- a/coreapi/chat_file_transfer.c +++ b/coreapi/chat_file_transfer.c @@ -48,8 +48,12 @@ static void _release_http_request(LinphoneChatMessage* msg) { } belle_sip_object_unref(msg->http_request); msg->http_request = NULL; - // unhold the reference on the message - linphone_chat_message_unref(msg); + if (msg->http_listener){ + belle_sip_object_unref(msg->http_listener); + msg->http_listener = NULL; + // unhold the reference that the listener was holding on the message + linphone_chat_message_unref(msg); + } } } @@ -121,7 +125,7 @@ static int linphone_chat_message_file_transfer_on_send_body(belle_sip_user_body_ if (offset + *size < linphone_content_get_size(msg->file_transfer_information)) { *size -= (*size % 16); } - plainBuffer = (char *)malloc(*size); + plainBuffer = (char *)ms_malloc0(*size); } /* get data from call back */ @@ -146,7 +150,7 @@ static int linphone_chat_message_file_transfer_on_send_body(belle_sip_user_body_ lime_encryptFile(linphone_content_get_cryptoContext_address(msg->file_transfer_information), (unsigned char *)linphone_content_get_key(msg->file_transfer_information), *size, plainBuffer, (char *)buffer); - free(plainBuffer); + ms_free(plainBuffer); /* check if we reach the end of file */ if (offset + *size >= linphone_content_get_size(msg->file_transfer_information)) { /* conclude file ciphering by calling it context with a zero size */ @@ -254,7 +258,7 @@ static void linphone_chat_message_process_response_from_post_file(void *data, one */ /* convert key to base64 */ int b64Size = b64_encode(NULL, FILE_TRANSFER_KEY_SIZE, NULL, 0); - char *keyb64 = (char *)malloc(b64Size + 1); + char *keyb64 = (char *)ms_malloc0(b64Size + 1); int xmlStringLength; b64Size = b64_encode(linphone_content_get_key(msg->file_transfer_information), @@ -264,7 +268,7 @@ static void linphone_chat_message_process_response_from_post_file(void *data, /* add the node containing the key to the file-info node */ xmlNewTextChild(cur, NULL, (const xmlChar *)"file-key", (const xmlChar *)keyb64); xmlFree(typeAttribute); - free(keyb64); + ms_free(keyb64); /* look for the file-name node and update its content */ while (fileInfoNodeChildren != NULL) { @@ -338,7 +342,7 @@ static void on_recv_body(belle_sip_user_body_handler_t *bh, belle_sip_message_t if (linphone_content_get_key(msg->file_transfer_information) != NULL) { /* we have a key, we must decrypt the file */ /* get data from callback to a plainBuffer */ - char *plainBuffer = (char *)malloc(size); + char *plainBuffer = (char *)ms_malloc0(size); lime_decryptFile(linphone_content_get_cryptoContext_address(msg->file_transfer_information), (unsigned char *)linphone_content_get_key(msg->file_transfer_information), size, plainBuffer, (char *)buffer); @@ -350,7 +354,7 @@ static void on_recv_body(belle_sip_user_body_handler_t *bh, belle_sip_message_t /* legacy: call back given by application level */ linphone_core_notify_file_transfer_recv(lc, msg, msg->file_transfer_information, plainBuffer, size); } - free(plainBuffer); + ms_free(plainBuffer); } else { /* regular file, no deciphering */ if (linphone_chat_message_cbs_get_file_transfer_recv(msg->callbacks)) { LinphoneBuffer *lb = linphone_buffer_new_from_data(buffer, size); @@ -454,13 +458,11 @@ static void linphone_chat_process_response_from_get_file(void *data, const belle } else { ms_warning("Unhandled HTTP code response %d for file transfer", code); } - ms_error("printf %d", code); _release_http_request(msg); } } void _linphone_chat_room_start_http_transfer(LinphoneChatMessage *msg, const char* url, const char* action, const belle_http_request_listener_callbacks_t *cbs) { - belle_http_request_listener_t *l; belle_generic_uri_t *uri = NULL; char* ua; @@ -486,8 +488,8 @@ void _linphone_chat_room_start_http_transfer(LinphoneChatMessage *msg, const cha belle_sip_object_ref(msg->http_request); /* give msg to listener to be able to start the actual file upload when server answer a 204 No content */ - l = belle_http_request_listener_create_from_callbacks(cbs, linphone_chat_message_ref(msg)); - belle_http_provider_send_request(msg->chat_room->lc->http_provider, msg->http_request, l); + msg->http_listener = belle_http_request_listener_create_from_callbacks(cbs, linphone_chat_message_ref(msg)); + belle_http_provider_send_request(msg->chat_room->lc->http_provider, msg->http_request, msg->http_listener); return; error: if (uri) { @@ -552,7 +554,7 @@ LinphoneChatMessage *linphone_chat_room_create_file_transfer_message(LinphoneCha msg->file_transfer_information = linphone_content_copy(initial_content); msg->dir = LinphoneChatMessageOutgoing; linphone_chat_message_set_to(msg, linphone_chat_room_get_peer_address(cr)); - linphone_chat_message_set_from(msg, linphone_address_new(linphone_core_get_identity(cr->lc))); + msg->from = linphone_address_new(linphone_core_get_identity(cr->lc)); /*direct assignment*/ /* this will be set to application/vnd.gsma.rcs-ft-http+xml when we will transfer the xml reply from server to the peers */ msg->content_type = NULL; /* this will store the http request during file upload to the server */ diff --git a/coreapi/linphonecore.c b/coreapi/linphonecore.c index 4cdf3eb54..c85f589c0 100644 --- a/coreapi/linphonecore.c +++ b/coreapi/linphonecore.c @@ -6299,6 +6299,9 @@ static void linphone_core_uninit(LinphoneCore *lc) if (lc->friends) /* FIXME we should wait until subscription to complete*/ ms_list_for_each(lc->friends,(void (*)(void *))linphone_friend_close_subscriptions); + + lc->chatrooms = ms_list_free_with_data(lc->chatrooms, (MSIterateFunc)linphone_chat_room_release); + linphone_core_set_state(lc,LinphoneGlobalShutdown,"Shutting down"); #ifdef VIDEO_ENABLED if (lc->previewstream!=NULL){ @@ -6327,9 +6330,6 @@ static void linphone_core_uninit(LinphoneCore *lc) } #endif //BUILD_UPNP - ms_list_for_each(lc->chatrooms, (MSIterateFunc)linphone_chat_room_release); - lc->chatrooms = ms_list_free(lc->chatrooms); - if (lp_config_needs_commit(lc->config)) lp_config_sync(lc->config); lp_config_destroy(lc->config); lc->config = NULL; /* Mark the config as NULL to block further calls */ diff --git a/coreapi/private.h b/coreapi/private.h index dc2e9549e..1e8499ab5 100644 --- a/coreapi/private.h +++ b/coreapi/private.h @@ -223,6 +223,7 @@ struct _LinphoneChatMessage { LinphoneContent *file_transfer_information; /**< used to store file transfer information when the message is of file transfer type */ char *content_type; /**< is used to specified the type of message to be sent, used only for file transfer message */ belle_http_request_t *http_request; /**< keep a reference to the http_request in case of file transfer in order to be able to cancel the transfer */ + belle_http_request_listener_t *http_listener; /* our listener, only owned by us*/ char *file_transfer_filepath; }; diff --git a/tester/liblinphone_tester.c b/tester/liblinphone_tester.c index edf00e0b7..15a3e8001 100644 --- a/tester/liblinphone_tester.c +++ b/tester/liblinphone_tester.c @@ -179,7 +179,8 @@ static const char* liblinphone_helper = "\t\t\t--domain \n" "\t\t\t--auth-domain \n" "\t\t\t--dns-hosts \n" - "\t\t\t--keep-recorded-files\n"; + "\t\t\t--keep-recorded-files\n" + "\t\t\t--disable-leak-detector\n"; int main (int argc, char *argv[]) { @@ -225,6 +226,8 @@ int main (int argc, char *argv[]) userhostsfile=argv[i]; } else if (strcmp(argv[i],"--keep-recorded-files")==0){ liblinphone_tester_keep_recorded_files(TRUE); + } else if (strcmp(argv[i],"--disable-leak-detector")==0){ + liblinphone_tester_disable_leak_detector(TRUE); } else { int bret = bc_tester_parse_args(argc, argv, i); if (bret>0) { diff --git a/tester/liblinphone_tester.h b/tester/liblinphone_tester.h index 7a6c9fb29..cf26134ff 100644 --- a/tester/liblinphone_tester.h +++ b/tester/liblinphone_tester.h @@ -79,6 +79,12 @@ extern void liblinphone_tester_keep_accounts( int keep ); **/ void liblinphone_tester_keep_recorded_files(int keep); +/** + * @brief Disable the automatic object leak detection. This is useful because the object leak detector prevents valgrind from seeing the leaks. + * @details By default object leak detector is enabled. +**/ +void liblinphone_tester_disable_leak_detector(int disabled); + /** * @brief Clears the created accounts during the testing session. */ diff --git a/tester/tester.c b/tester/tester.c index 2ea41327d..8e7908b3d 100644 --- a/tester/tester.c +++ b/tester/tester.c @@ -35,6 +35,7 @@ static bool_t liblinphone_tester_ipv6_enabled=FALSE; static int liblinphone_tester_keep_accounts_flag = 0; static int liblinphone_tester_keep_record_files = FALSE; +static int liblinphone_tester_leak_detector_disabled = FALSE; int manager_count = 0; int leaked_objects_count = 0; const MSAudioDiffParams audio_cmp_params = {10,2000}; @@ -415,6 +416,10 @@ void liblinphone_tester_keep_recorded_files(int keep){ liblinphone_tester_keep_record_files = keep; } +void liblinphone_tester_disable_leak_detector(int disabled){ + liblinphone_tester_leak_detector_disabled = disabled; +} + void liblinphone_tester_clear_accounts(void){ account_manager_destroy(); } @@ -484,17 +489,21 @@ int linphone_core_manager_get_mean_audio_up_bw(const LinphoneCoreManager *mgr) { } void liblinphone_tester_before_each() { - belle_sip_object_enable_leak_detector(TRUE); - leaked_objects_count = belle_sip_object_get_object_count(); + if (!liblinphone_tester_leak_detector_disabled){ + belle_sip_object_enable_leak_detector(TRUE); + leaked_objects_count = belle_sip_object_get_object_count(); + } } void liblinphone_tester_after_each() { - int leaked_objects = belle_sip_object_get_object_count() - leaked_objects_count; - // this will NOT be counted in tests fail but at least it will be shown - BC_ASSERT_EQUAL(leaked_objects, 0, int, "%d"); - if (leaked_objects > 0) { - belle_sip_object_dump_active_objects(); - ms_error("%d object%s leaked in latest test, please fix that!", leaked_objects, leaked_objects>1?"s were":"was"); + if (!liblinphone_tester_leak_detector_disabled){ + int leaked_objects = belle_sip_object_get_object_count() - leaked_objects_count; + // this will NOT be counted in tests fail but at least it will be shown + BC_ASSERT_EQUAL(leaked_objects, 0, int, "%d"); + if (leaked_objects > 0) { + belle_sip_object_dump_active_objects(); + ms_error("%d object%s leaked in latest test, please fix that!", leaked_objects, leaked_objects>1?"s were":"was"); + } } if (manager_count != 0) {