From d7b6b127bf8c655e5506d8dfdce074defec23737 Mon Sep 17 00:00:00 2001 From: jaybeepee Date: Mon, 29 Feb 2016 14:25:21 +0200 Subject: [PATCH] modules/ims_usrloc_scscf: delayed deletion of contacts on delete or expiry --- modules/ims_usrloc_scscf/dlist.c | 2 +- modules/ims_usrloc_scscf/impurecord.c | 47 ++++++++++++++------ modules/ims_usrloc_scscf/udomain.c | 63 +++++++++++++++++++-------- modules/ims_usrloc_scscf/usrloc.h | 5 ++- 4 files changed, 83 insertions(+), 34 deletions(-) diff --git a/modules/ims_usrloc_scscf/dlist.c b/modules/ims_usrloc_scscf/dlist.c index bfaf50d9012..f78a8d057d6 100644 --- a/modules/ims_usrloc_scscf/dlist.c +++ b/modules/ims_usrloc_scscf/dlist.c @@ -105,7 +105,7 @@ static inline int get_all_mem_ucontacts(void *buf, int len, unsigned int flags, void *cp; int shortage; int needed; - int i,j; + int i,j=0; cp = buf; shortage = 0; /* Reserve space for terminating 0000 */ diff --git a/modules/ims_usrloc_scscf/impurecord.c b/modules/ims_usrloc_scscf/impurecord.c index 5410d0687a5..b8f0b93c960 100644 --- a/modules/ims_usrloc_scscf/impurecord.c +++ b/modules/ims_usrloc_scscf/impurecord.c @@ -478,7 +478,6 @@ int insert_scontact(impurecord_t* _r, str* _contact, ucontact_info_t* _ci, ucont case 2://overwrite oldest LM_DBG("Too many contacts already registered, overwriting oldest for IMPU <%.*s>\n", _r->public_identity.len, _r->public_identity.s); //we can just remove the first one seeing the contacts are ordered on insertion with newest last and oldest first - mem_delete_ucontact(_r->newcontacts[0]); break; default://unknown LM_ERR("unknown maxcontact behaviour..... ignoring\n"); @@ -572,7 +571,7 @@ static inline struct ucontact* contact_match(unsigned int slot, str* _c) { ucontact_t* ptr = contact_list->slot[slot].first; while (ptr) { - if ((_c->len == ptr->c.len) && !memcmp(_c->s, ptr->c.s, _c->len)) {//check validity + if ((ptr->state != CONTACT_DELAYED_DELETE) && (_c->len == ptr->c.len) && !memcmp(_c->s, ptr->c.s, _c->len)) {//check validity return ptr; } ptr = ptr->next; @@ -593,7 +592,9 @@ static inline struct ucontact* contact_port_ip_match(unsigned int slot, str* _c) while (ptr) { aor_to_contact(&ptr->c, &contact_ip_port); //strip userpart from contact - if ((string_ip_port.len == contact_ip_port.len) && !memcmp(string_ip_port.s, contact_ip_port.s, string_ip_port.len)) { + if ((ptr->state != CONTACT_DELAYED_DELETE) + && (string_ip_port.len == contact_ip_port.len) + && !memcmp(string_ip_port.s, contact_ip_port.s, string_ip_port.len)) { return ptr; } @@ -614,7 +615,8 @@ static inline struct ucontact* contact_callid_match(unsigned int slot, ucontact_t* ptr = contact_list->slot[slot].first; while (ptr) { - if ((_c->len == ptr->c.len) && (_callid->len == ptr->callid.len) + if ((ptr->state != CONTACT_DELAYED_DELETE) + && (_c->len == ptr->c.len) && (_callid->len == ptr->callid.len) && !memcmp(_c->s, ptr->c.s, _c->len) && !memcmp(_callid->s, ptr->callid.s, _callid->len)) { return ptr; @@ -638,7 +640,8 @@ static inline struct ucontact* contact_path_match(unsigned int slot, str* _c, st if (_path == NULL) return contact_match(slot, _c); while (ptr) { - if ((_c->len == ptr->c.len) && (_path->len == ptr->path.len) + if ((ptr->state != CONTACT_DELAYED_DELETE) + && (_c->len == ptr->c.len) && (_path->len == ptr->path.len) && !memcmp(_c->s, ptr->c.s, _c->len) && !memcmp(_path->s, ptr->path.s, _path->len) && VALID_CONTACT(ptr, act_time) @@ -1012,7 +1015,7 @@ int link_contact_to_impu(impurecord_t* impu, ucontact_t* contact, int write_to_d while (i < MAX_CONTACTS_PER_IMPU && ptr) { if (ptr == contact) { - LM_DBG("contact [%.*s] already linked to impu [%.*s]\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s); + LM_DBG("contact [%p] => [%.*s] already linked to impu [%.*s] at position [%i]\n", contact, contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s, i); return 0; } i++; @@ -1027,9 +1030,10 @@ int link_contact_to_impu(impurecord_t* impu, ucontact_t* contact, int write_to_d if (i < MAX_CONTACTS_PER_IMPU) { LM_DBG("contact [%.*s] needs to be linked to impu [%.*s] at position %d\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s, i); - if (overwrite) - unlink_contact_from_impu(impu, impu->newcontacts[i], write_to_db, 0 /*implicit dereg of contact */); //unlink the contact we are overwriting - + if (overwrite) { + LM_DBG("In overwrite mode: going to unlink [%p] => [%.*s]\n", impu->newcontacts[i], impu->newcontacts[i]->c.len, impu->newcontacts[i]->c.s); + unlink_contact_from_impu(impu, impu->newcontacts[i], write_to_db, 0 /*implicit dereg of contact */); //unlink the contact we are overwriting + } impu->num_contacts = i + 1; //we always bump this - as unlink (in overwrite would have decremented) impu->newcontacts[i] = contact; ref_contact_unsafe(contact); @@ -1051,8 +1055,9 @@ int unlink_contact_from_impu(impurecord_t* impu, ucontact_t* contact, int write_ i = 0; int found = 0; ptr = impu->newcontacts[i]; + int locked = 0; - LM_DBG("asked to unlink contact [%.*s] from impu [%.*s]\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s); + LM_DBG("asked to unlink contact [%p] => [%.*s] from impu [%.*s]\n", contact, contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s); while (i < MAX_CONTACTS_PER_IMPU && ptr) { if (found) { @@ -1060,7 +1065,9 @@ int unlink_contact_from_impu(impurecord_t* impu, ucontact_t* contact, int write_ impu->newcontacts[i - 1] = impu->newcontacts[i]; } else { if (ptr == contact) { - LM_DBG("unlinking contact [%.*s] from impu [%.*s]\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s); + LM_DBG("unlinking contact [%p] => [%.*s] from impu [%.*s] at position [%i]\n", + contact, + contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s, i); found = 1; found_contact = ptr; impu->newcontacts[i] = 0; @@ -1073,14 +1080,28 @@ int unlink_contact_from_impu(impurecord_t* impu, ucontact_t* contact, int write_ if (found) { if (i < MAX_CONTACTS_PER_IMPU) { - LM_DBG("zero'ing last pointer to contact in the list\n"); + LM_DBG("zero'ing last pointer to contact in the list at position [%i]\n", i-1); impu->newcontacts[i - 1] = 0; } if (write_to_db && db_mode == WRITE_THROUGH && db_unlink_contact_from_impu(impu, found_contact) != 0) { LM_ERR("Failed to un-link DB contact [%.*s] from IMPU [%.*s]...continuing but db will be out of sync!\n", found_contact->c.len, found_contact->c.s, impu->public_identity.len, impu->public_identity.s); } - unref_contact_unsafe(found_contact); //should we lock the actual contact? safe version maybe? + + locked = lock_try(found_contact->lock); + if (locked == 0) { +// found_contact->state = CONTACT_DELAYED_DELETE; + unref_contact_unsafe(found_contact); //we don't unref because we don't have the lock on this particular contacts contact slot and we can't take it coz of deadlock. - so let + //a housekeeper thread do it + locked = 1; + } else { + LM_ERR("Could not get lock to remove link from of contact from impu...."); + //TODO: we either need to wait and retry or we need to get another process to do this for us.... right now we will leak a contact. + } + if (locked == 1) { + lock_release(found_contact->lock); + } + } else { LM_DBG("contact [%.*s] did not exist in IMPU list [%.*s] while trying to unlink\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s); } diff --git a/modules/ims_usrloc_scscf/udomain.c b/modules/ims_usrloc_scscf/udomain.c index ed36c53582a..94d71da1a6f 100644 --- a/modules/ims_usrloc_scscf/udomain.c +++ b/modules/ims_usrloc_scscf/udomain.c @@ -261,11 +261,13 @@ void mem_timer_udomain(udomain_t* _d) { struct impurecord* ptr, *t; struct ucontact* contact_ptr; unsigned int num_expired_contacts = 0; - int i, n, temp; + int i, n, temp, j; time_t now; + int abort = 0; + int slot; now = time(0); - int numcontacts = contact_list->max_collisions?(contact_list->max_collisions * contact_list->size):(contact_list->size); + int numcontacts = contact_list->size*2; //assume we should be ok for each slot to have 2 collisions if (expired_contacts_size < numcontacts) { LM_DBG("Changing expired_contacts list size from %d to %d\n", expired_contacts_size, numcontacts); if (expired_contacts){ @@ -273,7 +275,7 @@ void mem_timer_udomain(udomain_t* _d) { } expired_contacts = (ucontact_t**)pkg_malloc(numcontacts*sizeof(ucontact_t**)); if (!expired_contacts) { - LM_ERR("no more pkg mem\n"); + LM_ERR("no more pkg mem trying to allocate [%d] bytes\n", numcontacts*sizeof(ucontact_t**)); return; } expired_contacts_size = numcontacts; @@ -282,21 +284,36 @@ void mem_timer_udomain(udomain_t* _d) { //go through contacts first n = contact_list->max_collisions; LM_DBG("*** mem_timer_udomain - checking contacts - START ***\n"); - for (i = 0; i < contact_list->size; i++) { + for (i=0,j=0; i < contact_list->size; i++) { lock_contact_slot_i(i); contact_ptr = contact_list->slot[i].first; while (contact_ptr) { - LM_DBG("We have a contact in the new contact list in slot %d = [%.*s] (%.*s) which expires in %lf seconds and has a ref count of %d (state: %d)\n", - i, contact_ptr->aor.len, contact_ptr->aor.s, contact_ptr->c.len, contact_ptr->c.s, + if (num_expired_contacts >= numcontacts) { + LM_WARN("we don't have enough space to expire all contacts in this pass - will continue in next pass\n"); + abort = 1; + break; + } + LM_DBG("We have a [3gpp=%d] contact in the new contact list in slot %d = [%.*s] (%.*s) which expires in %lf seconds and has a ref count of %d (state: %d)\n", + contact_ptr->is_3gpp, i, contact_ptr->aor.len, contact_ptr->aor.s, contact_ptr->c.len, contact_ptr->c.s, (double) contact_ptr->expires - now, contact_ptr->ref_count, contact_ptr->state); //contacts are now deleted during impurecord processing - if ((contact_ptr->expires-now) <=0 && (contact_ptr->state != CONTACT_DELETED)) { - LM_DBG("expiring contact [%.*s].... setting to CONTACT_EXPIRE_PENDING_NOTIFY\n", contact_ptr->aor.len, contact_ptr->aor.s); - contact_ptr->state = CONTACT_EXPIRE_PENDING_NOTIFY; - ref_contact_unsafe(contact_ptr); - expired_contacts[num_expired_contacts] = contact_ptr; - num_expired_contacts++; + if ((contact_ptr->expires-now) <= 0) { + if (contact_ptr->state == CONTACT_DELAYED_DELETE) { + if (contact_ptr->ref_count <= 0) { + LM_DBG("contact in state CONTACT_DELATED_DELETE is about to be deleted"); + expired_contacts[num_expired_contacts] = contact_ptr; + num_expired_contacts++; + } else { + LM_DBG("contact in state CONTACT_DELATED_DELETE still has a ref count of [%d]... not doing anything for now\n", contact_ptr->ref_count); + } + } else if (contact_ptr->state != CONTACT_DELETED) { + LM_DBG("expiring contact [%.*s].... setting to CONTACT_EXPIRE_PENDING_NOTIFY\n", contact_ptr->aor.len, contact_ptr->aor.s); + contact_ptr->state = CONTACT_EXPIRE_PENDING_NOTIFY; + ref_contact_unsafe(contact_ptr); + expired_contacts[num_expired_contacts] = contact_ptr; + num_expired_contacts++; + } } contact_ptr = contact_ptr->next; } @@ -305,6 +322,9 @@ void mem_timer_udomain(udomain_t* _d) { } unlock_contact_slot_i(i); contact_list->max_collisions = n; + if (abort == 1) { + break; + } } LM_DBG("*** mem_timer_udomain - checking contacts - FINISHED ***\n"); @@ -350,11 +370,17 @@ void mem_timer_udomain(udomain_t* _d) { /* now we delete the expired contacts. (mark them for deletion */ for (i=0; isl); - LM_DBG("Setting contact state to CONTACT_DELETED for contact [%.*s]\n", expired_contacts[i]->aor.len, expired_contacts[i]->aor.s); - expired_contacts[i]->state = CONTACT_DELETED; - unref_contact_unsafe(expired_contacts[i]); - unlock_contact_slot_i(expired_contacts[i]->sl); + slot = expired_contacts[i]->sl; + lock_contact_slot_i(slot); + if (expired_contacts[i]->state != CONTACT_DELAYED_DELETE) { + LM_DBG("Setting contact state to CONTACT_DELETED for contact [%.*s]\n", expired_contacts[i]->aor.len, expired_contacts[i]->aor.s); + expired_contacts[i]->state = CONTACT_DELETED; + unref_contact_unsafe(expired_contacts[i]); + } else { + LM_DBG("deleting contact [%.*s]\n", expired_contacts[i]->aor.len, expired_contacts[i]->aor.s); + delete_scontact(expired_contacts[i]); + } + unlock_contact_slot_i(slot); } } @@ -803,6 +829,7 @@ void unref_contact_unsafe(ucontact_t* c) { if (c->ref_count < 0) { LM_WARN("reference dropped below zero... this should not happen\n"); } - delete_scontact(c); + c->state = CONTACT_DELAYED_DELETE; +// delete_scontact(c); } } diff --git a/modules/ims_usrloc_scscf/usrloc.h b/modules/ims_usrloc_scscf/usrloc.h index 2f7c0b9be8b..32b47a298df 100644 --- a/modules/ims_usrloc_scscf/usrloc.h +++ b/modules/ims_usrloc_scscf/usrloc.h @@ -145,11 +145,12 @@ typedef enum contact_state { CONTACT_VALID, CONTACT_DELETE_PENDING, CONTACT_EXPIRE_PENDING_NOTIFY, - CONTACT_DELETED + CONTACT_DELETED, + CONTACT_DELAYED_DELETE } contact_state_t; /*! \brief Valid contact is a contact that either didn't expire yet or is permanent */ -#define VALID_CONTACT(c, t) (((c->expires>t) || (c->expires==0)) && c->state!=CONTACT_DELETED && c->state!=CONTACT_DELETE_PENDING && c->state!=CONTACT_EXPIRE_PENDING_NOTIFY) +#define VALID_CONTACT(c, t) (((c->expires>t) || (c->expires==0)) && c->state!=CONTACT_DELETED && c->state!=CONTACT_DELETE_PENDING && c->state!=CONTACT_EXPIRE_PENDING_NOTIFY && c->state!=CONTACT_DELAYED_DELETE) #define VALID_UE_TYPE(c, t) ((t==0) || (t==1 && c->is_3gpp) || (t==2 && !c->is_3gpp))