From d04be099a2a54e36dd54209b03dc13f052362b5c Mon Sep 17 00:00:00 2001 From: Simon Morlat Date: Fri, 30 Sep 2016 00:39:17 +0200 Subject: [PATCH] Robustize LinphoneEvent api, fix memory leaks and crashes in error conditions. Add new tests. --- coreapi/bellesip_sal/sal_op_events.c | 33 +++---- coreapi/callbacks.c | 5 +- coreapi/event.c | 23 +++-- coreapi/friend.c | 2 +- include/sal/sal.h | 2 +- mediastreamer2 | 2 +- tester/eventapi_tester.c | 130 +++++++++++++++++++++++++-- tester/flexisip_tester.c | 4 +- tester/liblinphone_tester.h | 2 +- 9 files changed, 168 insertions(+), 35 deletions(-) diff --git a/coreapi/bellesip_sal/sal_op_events.c b/coreapi/bellesip_sal/sal_op_events.c index 8d8867383..1d03c09a5 100644 --- a/coreapi/bellesip_sal/sal_op_events.c +++ b/coreapi/bellesip_sal/sal_op_events.c @@ -46,15 +46,15 @@ static void subscribe_refresher_listener (belle_sip_refresher_t* refresher if (status_code==200) sss=SalSubscribeActive; else if (status_code==202) sss=SalSubscribePending; set_or_update_dialog(op,belle_sip_transaction_get_dialog(tr)); - op->base.root->callbacks.subscribe_response(op,sss); + op->base.root->callbacks.subscribe_response(op,sss, will_retry); } else if (status_code >= 300) { SalReason reason = SalReasonUnknown; if (status_code == 503) { /*refresher returns 503 for IO error*/ reason = SalReasonIOError; } sal_error_info_set(&op->error_info,reason,status_code,reason_phrase,NULL); - op->base.root->callbacks.subscribe_response(op,sss); - }if (status_code==0){ + op->base.root->callbacks.subscribe_response(op,sss, will_retry); + }else if (status_code==0){ op->base.root->callbacks.on_expire(op); } @@ -83,17 +83,19 @@ static void subscribe_process_dialog_terminated(void *ctx, const belle_sip_dialo belle_sip_dialog_t *dialog = belle_sip_dialog_terminated_event_get_dialog(event); SalOp* op= (SalOp*)ctx; if (op->dialog) { - op->dialog=NULL; - if (!belle_sip_dialog_is_server(dialog) && belle_sip_dialog_terminated_event_is_expired(event)){ - /*notify the app that our subscription is dead*/ - const char *eventname = NULL; - if (op->event){ - eventname = belle_sip_header_event_get_package_name(op->event); + if (belle_sip_dialog_terminated_event_is_expired(event)){ + if (!belle_sip_dialog_is_server(dialog)){ + /*notify the app that our subscription is dead*/ + const char *eventname = NULL; + if (op->event){ + eventname = belle_sip_header_event_get_package_name(op->event); + } + op->base.root->callbacks.notify(op, SalSubscribeTerminated, eventname, NULL); + }else{ + op->base.root->callbacks.incoming_subscribe_closed(op); } - op->base.root->callbacks.notify(op, SalSubscribeTerminated, eventname, NULL); } - sal_op_unref(op); - + set_or_update_dialog(op, NULL); } } @@ -167,6 +169,7 @@ static void subscribe_process_request_event(void *op_base, const belle_sip_reque belle_sip_response_t* resp; const char *eventname=NULL; const char *method=belle_sip_request_get_method(req); + belle_sip_dialog_t *dialog = NULL; belle_sip_object_ref(server_transaction); if (op->pending_server_trans) belle_sip_object_unref(op->pending_server_trans); @@ -190,14 +193,14 @@ static void subscribe_process_request_event(void *op_base, const belle_sip_reque if (!op->dialog) { if (strcmp(method,"SUBSCRIBE")==0){ - op->dialog = belle_sip_provider_create_dialog(op->base.root->prov,BELLE_SIP_TRANSACTION(server_transaction)); - if (!op->dialog){ + dialog = belle_sip_provider_create_dialog(op->base.root->prov,BELLE_SIP_TRANSACTION(server_transaction)); + if (!dialog){ resp=sal_op_create_response_from_request(op,req,481); belle_sip_server_transaction_send_response(server_transaction,resp); sal_op_release(op); return; } - belle_sip_dialog_set_application_data(op->dialog, sal_op_ref(op)); + set_or_update_dialog(op, dialog); ms_message("new incoming subscription from [%s] to [%s]",sal_op_get_from(op),sal_op_get_to(op)); }else{ /*this is a NOTIFY*/ handle_notify(op, req, eventname, (SalBodyHandler *)body_handler); diff --git a/coreapi/callbacks.c b/coreapi/callbacks.c index bc701782d..a82149d7f 100644 --- a/coreapi/callbacks.c +++ b/coreapi/callbacks.c @@ -1315,9 +1315,8 @@ static void info_received(SalOp *op, SalBodyHandler *body_handler){ linphone_core_notify_info_message(lc,op,body_handler); } -static void subscribe_response(SalOp *op, SalSubscribeStatus status){ +static void subscribe_response(SalOp *op, SalSubscribeStatus status, int will_retry){ LinphoneEvent *lev=(LinphoneEvent*)sal_op_get_user_pointer(op); - const SalErrorInfo *ei=sal_op_get_error_info(op); if (lev==NULL) return; @@ -1326,7 +1325,7 @@ static void subscribe_response(SalOp *op, SalSubscribeStatus status){ }else if (status==SalSubscribePending){ linphone_event_set_state(lev,LinphoneSubscriptionPending); }else{ - if (lev->subscription_state==LinphoneSubscriptionActive && (ei->reason==SalReasonIOError || ei->reason == SalReasonNoMatch)){ + if (will_retry){ linphone_event_set_state(lev,LinphoneSubscriptionOutgoingProgress); } else linphone_event_set_state(lev,LinphoneSubscriptionError); diff --git a/coreapi/event.c b/coreapi/event.c index 506ed35ca..f4f69b7a5 100644 --- a/coreapi/event.c +++ b/coreapi/event.c @@ -68,6 +68,15 @@ LINPHONE_PUBLIC const char *linphone_publish_state_to_string(LinphonePublishStat return NULL; } +static void linphone_event_release(LinphoneEvent *lev){ + if (lev->op) { + /*this will stop the refreesher*/ + sal_op_release(lev->op); + lev->op = NULL; + } + linphone_event_unref(lev); +} + static LinphoneEvent * linphone_event_new_base(LinphoneCore *lc, LinphoneSubscriptionDir dir, const char *name, SalOp *op){ LinphoneEvent *lev=belle_sip_object_new(LinphoneEvent); lev->lc=lc; @@ -112,7 +121,7 @@ void linphone_event_set_state(LinphoneEvent *lev, LinphoneSubscriptionState stat lev->subscription_state=state; linphone_core_notify_subscription_state_changed(lev->lc,lev,state); if (state==LinphoneSubscriptionTerminated || state == LinphoneSubscriptionError){ - linphone_event_unref(lev); + linphone_event_release(lev); } } } @@ -124,13 +133,17 @@ void linphone_event_set_publish_state(LinphoneEvent *lev, LinphonePublishState s linphone_core_notify_publish_state_changed(lev->lc,lev,state); switch(state){ case LinphonePublishCleared: - if (lev->expires!=-1) - linphone_event_unref(lev); + if (lev->expires!=-1){ + linphone_event_release(lev); + } break; case LinphonePublishOk: + if (lev->expires==-1){ + linphone_event_release(lev); + } + break; case LinphonePublishError: - if (lev->expires==-1) - linphone_event_unref(lev); + linphone_event_release(lev); break; case LinphonePublishNone: case LinphonePublishProgress: diff --git a/coreapi/friend.c b/coreapi/friend.c index 4a7d97015..83d4c7b8f 100644 --- a/coreapi/friend.c +++ b/coreapi/friend.c @@ -679,7 +679,7 @@ BuddyInfo * linphone_friend_get_info(const LinphoneFriend *lf){ } /* - * updates the subscriptions. + * updates the p2p subscriptions. * If only_when_registered is TRUE, subscribe will be sent only if the friend's corresponding proxy config is in registered. * Otherwise if the proxy config goes to unregistered state, the subscription refresh will be suspended. * An optional proxy whose state has changed can be passed to optimize the processing. diff --git a/include/sal/sal.h b/include/sal/sal.h index bf8aadf8b..3f8de46fe 100644 --- a/include/sal/sal.h +++ b/include/sal/sal.h @@ -502,7 +502,7 @@ typedef void (*SalOnTextReceived)(SalOp *op, const SalMessage *msg); typedef void (*SalOnTextDeliveryUpdate)(SalOp *op, SalTextDeliveryStatus); typedef void (*SalOnIsComposingReceived)(SalOp *op, const SalIsComposing *is_composing); typedef void (*SalOnNotifyRefer)(SalOp *op, SalReferStatus state); -typedef void (*SalOnSubscribeResponse)(SalOp *op, SalSubscribeStatus status); +typedef void (*SalOnSubscribeResponse)(SalOp *op, SalSubscribeStatus status, int will_retry); typedef void (*SalOnNotify)(SalOp *op, SalSubscribeStatus status, const char *event, SalBodyHandler *body); typedef void (*SalOnSubscribeReceived)(SalOp *salop, const char *event, const SalBodyHandler *body); typedef void (*SalOnIncomingSubscribeClosed)(SalOp *salop); diff --git a/mediastreamer2 b/mediastreamer2 index ed6b331f6..26f884bf9 160000 --- a/mediastreamer2 +++ b/mediastreamer2 @@ -1 +1 @@ -Subproject commit ed6b331f622f7ffd976e3faf0007c577d6f31eee +Subproject commit 26f884bf977977041fe6f98a0af186be1580bf22 diff --git a/tester/eventapi_tester.c b/tester/eventapi_tester.c index 6baf10504..e98e40c70 100644 --- a/tester/eventapi_tester.c +++ b/tester/eventapi_tester.c @@ -72,8 +72,8 @@ void linphone_subscription_state_change(LinphoneCore *lc, LinphoneEvent *lev, Li else linphone_event_deny_subscription(lev, LinphoneReasonDeclined); break; - case LinphoneSubscriptionOutgoingInit: - counters->number_of_LinphoneSubscriptionOutgoingInit++; + case LinphoneSubscriptionOutgoingProgress: + counters->number_of_LinphoneSubscriptionOutgoingProgress++; break; case LinphoneSubscriptionPending: counters->number_of_LinphoneSubscriptionPending++; @@ -142,7 +142,7 @@ static void subscribe_test_declined(void) { lev=linphone_core_subscribe(marie->lc,pauline->identity,"dodo",600,content); linphone_event_ref(lev); - BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingInit,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionIncomingReceived,1,3000)); BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionError,1,21000));/*yes flexisip may wait 20 secs in case of forking*/ ei=linphone_event_get_error_info(lev); @@ -153,6 +153,7 @@ static void subscribe_test_declined(void) { } BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionTerminated,1,1000)); + bctbx_list_free(lcs); linphone_content_unref(content); linphone_event_unref(lev); linphone_core_manager_destroy(marie); @@ -186,7 +187,7 @@ static void subscribe_test_with_args(bool_t terminated_by_subscriber, RefreshTes lev=linphone_core_subscribe(marie->lc,pauline->identity,"dodo",expires,content); - BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingInit,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionIncomingReceived,1,3000)); BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,1,3000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionActive,1,1000)); @@ -213,6 +214,7 @@ static void subscribe_test_with_args(bool_t terminated_by_subscriber, RefreshTes BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionTerminated,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionTerminated,1,1000)); + bctbx_list_free(lcs); linphone_content_unref(content); linphone_core_manager_destroy(marie); linphone_core_manager_destroy(pauline); @@ -242,7 +244,7 @@ static void subscribe_test_with_args2(bool_t terminated_by_subscriber, RefreshTe linphone_event_add_custom_header(lev,"My-Header2","pimpon"); linphone_event_send_subscribe(lev,content); - BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingInit,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionIncomingReceived,1,3000)); if (pauline->stat.number_of_LinphoneSubscriptionIncomingReceived == 1) { @@ -278,6 +280,7 @@ static void subscribe_test_with_args2(bool_t terminated_by_subscriber, RefreshTe linphone_content_unref(content); linphone_core_manager_destroy(marie); linphone_core_manager_destroy(pauline); + bctbx_list_free(lcs); } static void subscribe_test_terminated_by_subscriber(void){ @@ -303,6 +306,119 @@ static void subscribe_test_manually_refreshed(void){ subscribe_test_with_args(TRUE,ManualRefresh); } +static void subscribe_loosing_dialog(void) { + LinphoneCoreManager* marie = linphone_core_manager_new( "marie_rc"); + LinphoneCoreManager* pauline = linphone_core_manager_new( "pauline_tcp_rc"); + LinphoneContent* content; + LinphoneEvent *lev; + int expires= 4; + bctbx_list_t* lcs=bctbx_list_append(NULL,marie->lc); + + lcs=bctbx_list_append(lcs,pauline->lc); + + content = linphone_core_create_content(marie->lc); + linphone_content_set_type(content,"application"); + linphone_content_set_subtype(content,"somexml"); + linphone_content_set_buffer(content,subscribe_content,strlen(subscribe_content)); + + lev=linphone_core_create_subscribe(marie->lc,pauline->identity,"dodo",expires); + linphone_event_add_custom_header(lev,"My-Header","pouet"); + linphone_event_add_custom_header(lev,"My-Header2","pimpon"); + linphone_event_send_subscribe(lev,content); + + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionIncomingReceived,1,3000)); + + + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,1,5000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionActive,1,5000)); + + /*make sure marie receives first notification before terminating*/ + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_NotifyReceived,1,5000)); + + /* now pauline looses internet connection and reboots */ + linphone_core_set_network_reachable(pauline->lc, FALSE); + /*let expire the incoming subscribe received by pauline */ + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionTerminated,1,5000)); + lcs = bctbx_list_remove(lcs, pauline->lc); + linphone_core_manager_destroy(pauline); + pauline = linphone_core_manager_new( "pauline_tcp_rc"); + lcs = bctbx_list_append(lcs, pauline->lc); + + /*marie will retry the subscription*/ + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,2,8000)); + /*and get it accepted again*/ + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,2,5000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionActive,1,5000)); + BC_ASSERT_EQUAL(linphone_event_get_subscription_state(pauline->lev), LinphoneSubscriptionActive, int, "%d"); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_NotifyReceived,2,5000)); + linphone_event_terminate(lev); + + + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionTerminated,1,5000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionTerminated,1,5000)); + + linphone_content_unref(content); + linphone_core_manager_destroy(marie); + linphone_core_manager_destroy(pauline); + bctbx_list_free(lcs); +} + +static void subscribe_with_io_error(void) { + LinphoneCoreManager* marie = linphone_core_manager_new( "marie_rc"); + LinphoneCoreManager* pauline = linphone_core_manager_new( "pauline_tcp_rc"); + LinphoneContent* content; + LinphoneEvent *lev; + int expires= 4; + bctbx_list_t* lcs=bctbx_list_append(NULL,marie->lc); + + lcs=bctbx_list_append(lcs,pauline->lc); + + content = linphone_core_create_content(marie->lc); + linphone_content_set_type(content,"application"); + linphone_content_set_subtype(content,"somexml"); + linphone_content_set_buffer(content,subscribe_content,strlen(subscribe_content)); + + lev=linphone_core_create_subscribe(marie->lc,pauline->identity,"dodo",expires); + linphone_event_add_custom_header(lev,"My-Header","pouet"); + linphone_event_add_custom_header(lev,"My-Header2","pimpon"); + linphone_event_send_subscribe(lev,content); + + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionIncomingReceived,1,3000)); + + + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,1,5000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionActive,1,5000)); + + /*make sure marie receives first notification before terminating*/ + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_NotifyReceived,1,5000)); + + /* now marie gets network errors when refreshing*/ + sal_set_send_error(marie->lc->sal, -1); + + /*marie will retry the subscription*/ + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,2,8000)); + sal_set_send_error(marie->lc->sal, 0); + + /*and get it accepted again*/ + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,2,10000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionActive,2,5000)); + BC_ASSERT_EQUAL(linphone_event_get_subscription_state(pauline->lev), LinphoneSubscriptionActive, int, "%d"); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_NotifyReceived,2,5000)); + linphone_event_terminate(lev); + + + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionTerminated,1,5000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionTerminated,1,5000)); + + linphone_content_unref(content); + linphone_core_manager_destroy(marie); + linphone_core_manager_destroy(pauline); + bctbx_list_free(lcs); +} + + static void publish_test_with_args(bool_t refresh, int expires){ LinphoneCoreManager* marie = linphone_core_manager_new( "marie_rc"); LinphoneCoreManager* pauline = linphone_core_manager_new( "pauline_tcp_rc"); @@ -392,8 +508,10 @@ test_t event_tests[] = { TEST_ONE_TAG("Subscribe terminated by subscriber", subscribe_test_terminated_by_subscriber, "presence"), TEST_ONE_TAG("Subscribe with custom headers", subscribe_test_with_custom_header, "presence"), TEST_ONE_TAG("Subscribe refreshed", subscribe_test_refreshed, "presence"), + TEST_ONE_TAG("Subscribe loosing dialog", subscribe_loosing_dialog, "presence"), + TEST_ONE_TAG("Subscribe with io error", subscribe_with_io_error, "presence"), TEST_ONE_TAG("Subscribe manually refreshed", subscribe_test_manually_refreshed, "presence"), - TEST_ONE_TAG("Subscribe terminated by notifier", subscribe_test_terminated_by_notifier, "LeaksMemory"), + TEST_ONE_TAG("Subscribe terminated by notifier", subscribe_test_terminated_by_notifier, "presence"), TEST_ONE_TAG("Publish", publish_test, "presence"), TEST_ONE_TAG("Publish without expires", publish_without_expires, "presence"), TEST_ONE_TAG("Publish without automatic refresh",publish_no_auto_test, "presence"), diff --git a/tester/flexisip_tester.c b/tester/flexisip_tester.c index dbccee2dd..7f68bb684 100644 --- a/tester/flexisip_tester.c +++ b/tester/flexisip_tester.c @@ -41,7 +41,7 @@ static void subscribe_forking(void) { lev=linphone_core_subscribe(marie->lc,pauline->identity,"dodo",expires,content); - BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingInit,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline->stat.number_of_LinphoneSubscriptionIncomingReceived,1,3000)); BC_ASSERT_TRUE(wait_for_list(lcs,&pauline2->stat.number_of_LinphoneSubscriptionIncomingReceived,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,1,1000)); @@ -1110,7 +1110,7 @@ static void test_list_subscribe (void) { linphone_event_send_subscribe(lev,content); - BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingInit,1,1000)); + BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionOutgoingProgress,1,1000)); BC_ASSERT_TRUE(wait_for_list(lcs,&marie->stat.number_of_LinphoneSubscriptionActive,1,5000)); /*make sure marie receives first notification before terminating*/ diff --git a/tester/liblinphone_tester.h b/tester/liblinphone_tester.h index b65543e38..fad82cc66 100644 --- a/tester/liblinphone_tester.h +++ b/tester/liblinphone_tester.h @@ -210,7 +210,7 @@ typedef struct _stats { LinphoneInfoMessage* last_received_info_message; int number_of_LinphoneSubscriptionIncomingReceived; - int number_of_LinphoneSubscriptionOutgoingInit; + int number_of_LinphoneSubscriptionOutgoingProgress; int number_of_LinphoneSubscriptionPending; int number_of_LinphoneSubscriptionActive; int number_of_LinphoneSubscriptionTerminated;