modules/ims_usrloc_scscf,ims_registrar_scscf,ims_qos: improvements, optimisations, cleanup

	- the module needed some cleanup as well as a possible deadlock situation which is now fixed

modules/ims_isc/checker.c

@@ -522,7 +522,6 @@ void isc_free_match(isc_match *m) {
 	}
 	LM_DBG("isc_match_free: match position freed\n");
 }
-
 /**
  *	Find if user is registered or not => TRUE/FALSE.
  * This uses the S-CSCF registrar to get the state.
@@ -530,33 +529,24 @@ void isc_free_match(isc_match *m) {
  * @returns the reg_state
  */
 int isc_is_registered(str *uri, udomain_t *d) {
-	int result = 0;
-
-	int ret = 0;
-	impurecord_t *p;
-
-	LM_DBG("locking domain\n");
-	isc_ulb.lock_udomain(d, uri);
-
-	LM_DBG("Searching in usrloc\n");
-	//need to get the urecord
-	if ((ret = isc_ulb.get_impurecord(d, uri, &p)) != 0) {
-		LM_DBG("no record exists for [%.*s]\n", uri->len, uri->s);
-		isc_ulb.unlock_udomain(d, uri);
-		return result;
-	}
-
-	LM_DBG("Finished searching usrloc\n");
-	if (p) {
-		result = p->reg_state;
-		//need to free the record somewhere
-//		isc_ulb.release_urecord(p);
-		//need to do an unlock on the domain somewhere
-		isc_ulb.unlock_udomain(d, uri);
+    int result = 0;
+    int ret = 0;
+    impurecord_t *p;
+
+    isc_ulb.lock_udomain(d, uri);
+
+    LM_DBG("Searching in usrloc\n");
+    //need to get the urecord
+    if ((ret = isc_ulb.get_impurecord(d, uri, &p)) != 0) {
+        LM_DBG("no record exists for [%.*s]\n", uri->len, uri->s);
+        isc_ulb.unlock_udomain(d, uri);
+        return result;
+    }
 
-	}
+    LM_DBG("Finished searching usrloc\n");
+    result = p->reg_state;
+    isc_ulb.unlock_udomain(d, uri);
 
-	isc_ulb.unlock_udomain(d, uri);
-	return result;
+    return result;
 }
 

modules/ims_registrar_scscf/cxdx_sar.c

@@ -198,7 +198,7 @@ void async_cdp_callback(int is_timeout, void *param, AAAMessage *saa, long elaps
                 goto error;
 
             case AAA_SUCCESS:
-                LM_DBG("received AAA success\n");
+                LM_DBG("received AAA success for SAR - SAA\n");
                 break;
 
             default:
@@ -224,7 +224,8 @@ void async_cdp_callback(int is_timeout, void *param, AAAMessage *saa, long elaps
                 rerrno = R_SAR_FAILED;
                 goto error;
             }
-            LM_DBG("Successfully parse user data XML\n");
+            LM_DBG("Successfully parse user data XML setting ref to 1 (we are referencing it)\n");
+            s->ref_count = 1; //no need to lock as nobody else will be referencing this piece of memory just yet
         } else {
             if (data->require_user_data) {
                 LM_ERR("We require User data for this assignment/register and none was supplied\n");
@@ -243,7 +244,7 @@ void async_cdp_callback(int is_timeout, void *param, AAAMessage *saa, long elaps
         }
 
         //here we update the contacts and also build the new contact header for the 200 OK reply
-        if (update_contacts_new(t->uas.request, data->domain, &data->public_identity, data->sar_assignment_type, &s, &ccf1, &ccf2, &ecf1, &ecf2, &data->contact_header) <= 0) {
+        if (update_contacts(t->uas.request, data->domain, &data->public_identity, data->sar_assignment_type, &s, &ccf1, &ccf2, &ecf1, &ecf2, &data->contact_header) <= 0) {
             LM_ERR("Error processing REGISTER\n");
             rerrno = R_SAR_FAILED;
             goto error;
@@ -267,6 +268,10 @@ done:
 
     create_return_code(result);
 
+    //release our reference on subscription (s)
+    if (s) 
+        ul.unref_subscription(s);
+    
     //free memory
     if (saa) cdpb.AAAFreeMessage(&saa);
     if (t) {
@@ -281,11 +286,10 @@ done:
     return;
 
 error:
+    create_return_code(-2);
     if (data->sar_assignment_type != AVP_IMS_SAR_UNREGISTERED_USER)
         reg_send_reply_transactional(t->uas.request, data->contact_header, t);
 		
-    create_return_code(-2);
-
 error_no_send: //if we don't have the transaction then we can't send a transaction response
     update_stat(rejected_registrations, 1);
     //free memory

modules/ims_registrar_scscf/lookup.c

@@ -68,96 +68,97 @@ int lookup(struct sip_msg* _m, udomain_t* _d) {
     flag_t old_bflags;
     int i = 0;
 
-	if (!_m){
-		LM_ERR("NULL message!!!\n");
-		return -1;
-	}
-
-	if (_m->new_uri.s) aor = _m->new_uri;
-	else aor = _m->first_line.u.request.uri;
-	
-	for(i=0;i<aor.len;i++)
-		if (aor.s[i]==';' || aor.s[i]=='?') {
-			aor.len = i;
-			break;
-		}
-
-	LM_DBG("Looking for <%.*s>\n",aor.len,aor.s);
+    if (!_m) {
+        LM_ERR("NULL message!!!\n");
+        return -1;
+    }
+
+    if (_m->new_uri.s) aor = _m->new_uri;
+    else aor = _m->first_line.u.request.uri;
+
+    for (i = 0; i < aor.len; i++)
+        if (aor.s[i] == ';' || aor.s[i] == '?') {
+            aor.len = i;
+            break;
+        }
+
+    LM_DBG("Looking for <%.*s>\n", aor.len, aor.s);
 
     get_act_time();
 
     ul.lock_udomain(_d, &aor);
     res = ul.get_impurecord(_d, &aor, &r);
-    if (res > 0) {
-	LM_DBG("'%.*s' Not found in usrloc\n", aor.len, ZSW(aor.s));
-	ul.unlock_udomain(_d, &aor);
-	return -1;
+    if (res != 0) {
+        LM_DBG("'%.*s' Not found in usrloc\n", aor.len, ZSW(aor.s));
+        ul.unlock_udomain(_d, &aor);
+        return -1;
     }
     ret = -1;
     i = 0;
 
     while (i < MAX_CONTACTS_PER_IMPU && (ptr = r->newcontacts[i])) {
-	if (VALID_CONTACT(ptr, act_time) && allowed_method(_m, ptr)) {
-	    LM_DBG("Found a valid contact [%.*s]\n", ptr->c.len, ptr->c.s);
-	    i++;
-	    break;
-	}
-	i++;
+        if (VALID_CONTACT(ptr, act_time) && allowed_method(_m, ptr)) {
+            LM_DBG("Found a valid contact [%.*s]\n", ptr->c.len, ptr->c.s);
+            i++;
+            break;
+        }
+        i++;
     }
 
     /* look first for an un-expired and supported contact */
     if (ptr == 0) {
-	LM_INFO("No contacts founds for IMPU <%.*s>\n",aor.len,aor.s);
-	/* nothing found */
-	goto done;
+        LM_INFO("No contacts founds for IMPU <%.*s>\n", aor.len, aor.s);
+        /* nothing found */
+        goto done;
     }
 
     ret = 1;
     if (ptr) {
-	if (rewrite_uri(_m, &ptr->c) < 0) {
-	    LM_ERR("unable to rewrite Request-URI\n");
-	    ret = -3;
-	    goto done;
-	}
-
-	/* reset next hop address */
-	reset_dst_uri(_m);
-
-	/* If a Path is present, use first path-uri in favour of
-	 * received-uri because in that case the last hop towards the uac
-	 * has to handle NAT. - agranig */
-	if (ptr->path.s && ptr->path.len) {
-	    if (get_path_dst_uri(&ptr->path, &path_dst) < 0) {
-		LM_ERR("failed to get dst_uri for Path\n");
-		ret = -3;
-		goto done;
-	    }
-	    if (set_path_vector(_m, &ptr->path) < 0) {
-		LM_ERR("failed to set path vector\n");
-		return -1;
-	    }
-	    if (set_dst_uri(_m, &path_dst) < 0) {
-		LM_ERR("failed to set dst_uri of Path\n");
-		ret = -3;
-		goto done;
-	    }
-	} else if (ptr->received.s && ptr->received.len) {
-	    if (set_dst_uri(_m, &ptr->received) < 0) {
-		ret = -3;
-		goto done;
-	    }
-	}
-
-	set_ruri_q(ptr->q);
-
-	old_bflags = 0;
-	getbflagsval(0, &old_bflags);
-	setbflagsval(0, old_bflags | ptr->cflags);
-
-	if (ptr->sock)
-	    set_force_socket(_m, ptr->sock);
-
-	ptr = ptr->next;
+        if (rewrite_uri(_m, &ptr->c) < 0) {
+            LM_ERR("unable to rewrite Request-URI\n");
+            ret = -3;
+            goto done;
+        }
+
+        /* reset next hop address */
+        reset_dst_uri(_m);
+
+        /* If a Path is present, use first path-uri in favour of
+         * received-uri because in that case the last hop towards the uac
+         * has to handle NAT. - agranig */
+        if (ptr->path.s && ptr->path.len) {
+            if (get_path_dst_uri(&ptr->path, &path_dst) < 0) {
+                LM_ERR("failed to get dst_uri for Path\n");
+                ret = -3;
+                goto done;
+            }
+            if (set_path_vector(_m, &ptr->path) < 0) {
+                LM_ERR("failed to set path vector\n");
+                ret = -1;
+                goto done;
+            }
+            if (set_dst_uri(_m, &path_dst) < 0) {
+                LM_ERR("failed to set dst_uri of Path\n");
+                ret = -3;
+                goto done;
+            }
+        } else if (ptr->received.s && ptr->received.len) {
+            if (set_dst_uri(_m, &ptr->received) < 0) {
+                ret = -3;
+                goto done;
+            }
+        }
+
+        set_ruri_q(ptr->q);
+
+        old_bflags = 0;
+        getbflagsval(0, &old_bflags);
+        setbflagsval(0, old_bflags | ptr->cflags);
+
+        if (ptr->sock)
+            set_force_socket(_m, ptr->sock);
+
+        ptr = ptr->next;
     }
 
     /* Append branches if enabled */
@@ -165,24 +166,24 @@ int lookup(struct sip_msg* _m, udomain_t* _d) {
 
     //the last i was the first valid contact we found - let's go through the rest of valid contacts and append the branches.
     while (i < MAX_CONTACTS_PER_IMPU && (ptr = r->newcontacts[i])) {
-	if (VALID_CONTACT(ptr, act_time) && allowed_method(_m, ptr)) {
-	    path_dst.len = 0;
-	    if (ptr->path.s && ptr->path.len
-		    && get_path_dst_uri(&ptr->path, &path_dst) < 0) {
-		LM_ERR("failed to get dst_uri for Path\n");
-		continue;
-	    }
-
-	    /* The same as for the first contact applies for branches
-	     * regarding path vs. received. */
-	    if (km_append_branch(_m, &ptr->c, path_dst.len ? &path_dst : &ptr->received,
-		    &ptr->path, ptr->q, ptr->cflags, ptr->sock) == -1) {
-		LM_ERR("failed to append a branch\n");
-		/* Also give a chance to the next branches*/
-		continue;
-	    }
-	}
-	i++;
+        if (VALID_CONTACT(ptr, act_time) && allowed_method(_m, ptr)) {
+            path_dst.len = 0;
+            if (ptr->path.s && ptr->path.len
+                    && get_path_dst_uri(&ptr->path, &path_dst) < 0) {
+                LM_ERR("failed to get dst_uri for Path\n");
+                continue;
+            }
+
+            /* The same as for the first contact applies for branches
+             * regarding path vs. received. */
+            if (km_append_branch(_m, &ptr->c, path_dst.len ? &path_dst : &ptr->received,
+                    &ptr->path, ptr->q, ptr->cflags, ptr->sock) == -1) {
+                LM_ERR("failed to append a branch\n");
+                /* Also give a chance to the next branches*/
+                continue;
+            }
+        }
+        i++;
     }
 
 done:
@@ -194,36 +195,42 @@ int lookup_path_to_contact(struct sip_msg* _m, char* contact_uri) {
     ucontact_t* contact;
     str s_contact_uri;
     str path_dst;
-    
+
+    get_act_time();
     if (get_str_fparam(&s_contact_uri, _m, (fparam_t*) contact_uri) < 0) {
-	    LM_ERR("failed to get RURI\n");
-	    return -1;
+        LM_ERR("failed to get RURI\n");
+        return -1;
     }
     LM_DBG("Looking up contact [%.*s]\n", s_contact_uri.len, s_contact_uri.s);
 
     if (ul.get_ucontact(NULL, &s_contact_uri, 0, 0, 0, &contact) == 0) { //get_contact returns with lock
-	LM_DBG("CONTACT FOUND and path is [%.*s]\n", contact->path.len, contact->path.s);
-	
-	if (get_path_dst_uri(&contact->path, &path_dst) < 0) {
-	    LM_ERR("failed to get dst_uri for Path\n");
-	    ul.release_ucontact(contact);
-	    return -1;
-	}
-	if (set_path_vector(_m, &contact->path) < 0) {
-	    LM_ERR("failed to set path vector\n");
-	    ul.release_ucontact(contact);
-	    return -1;
-	}
-	if (set_dst_uri(_m, &path_dst) < 0) {
-	    LM_ERR("failed to set dst_uri of Path\n");
-	    ul.release_ucontact(contact);
-	    return -1;
-	}
-	
-	ul.release_ucontact(contact);
-	return 1;
-    }
 
+        if (!VALID_CONTACT(contact, act_time)) {
+            LM_DBG("Contact is not valid...ignoring\n");
+            ul.release_ucontact(contact);
+        } else {
+            LM_DBG("CONTACT FOUND and path is [%.*s]\n", contact->path.len, contact->path.s);
+
+            if (get_path_dst_uri(&contact->path, &path_dst) < 0) {
+                LM_ERR("failed to get dst_uri for Path\n");
+                ul.release_ucontact(contact);
+                return -1;
+            }
+            if (set_path_vector(_m, &contact->path) < 0) {
+                LM_ERR("failed to set path vector\n");
+                ul.release_ucontact(contact);
+                return -1;
+            }
+            if (set_dst_uri(_m, &path_dst) < 0) {
+                LM_ERR("failed to set dst_uri of Path\n");
+                ul.release_ucontact(contact);
+                return -1;
+            }
+
+            ul.release_ucontact(contact);
+            return 1;
+        }
+    }
     LM_DBG("no contact found for [%.*s]\n", s_contact_uri.len, s_contact_uri.s);
     return -1;
 }
@@ -231,35 +238,34 @@ int lookup_path_to_contact(struct sip_msg* _m, char* contact_uri) {
 /*! \brief the impu_registered() function
  * Return true if the AOR in the To Header is registered
  */
-int impu_registered(struct sip_msg* _m, char* _t, char* _s)
-{
-	impurecord_t* r;
-	int res, ret=-1;
-
-	str impu;
-	impu = cscf_get_public_identity(_m);
-
-	LM_DBG("Looking for IMPU <%.*s>\n", impu.len, impu.s);
-
-	ul.lock_udomain((udomain_t*)_t, &impu);
-	res = ul.get_impurecord((udomain_t*)_t, &impu, &r);
-
-	if (res < 0) {
-		ul.unlock_udomain((udomain_t*)_t, &impu);
-		LM_ERR("failed to query usrloc for IMPU <%.*s>\n", impu.len, impu.s);
-		return ret;
-	}
-
-	if (res == 0) {
-		if (r->reg_state == IMPU_REGISTERED ) ret = 1;
-		ul.unlock_udomain((udomain_t*) _t, &impu);
-		LM_DBG("'%.*s' found in usrloc\n", impu.len, ZSW(impu.s));
-		return ret;
-	}
-
-	ul.unlock_udomain((udomain_t*)_t, &impu);
-	LM_DBG("'%.*s' not found in usrloc\n", impu.len, ZSW(impu.s));
-	return ret;
+int impu_registered(struct sip_msg* _m, char* _t, char* _s) {
+    impurecord_t* r;
+    int res, ret = -1;
+
+    str impu;
+    impu = cscf_get_public_identity(_m);
+
+    LM_DBG("Looking for IMPU <%.*s>\n", impu.len, impu.s);
+
+    ul.lock_udomain((udomain_t*) _t, &impu);
+    res = ul.get_impurecord((udomain_t*) _t, &impu, &r);
+
+    if (res < 0) {
+        ul.unlock_udomain((udomain_t*) _t, &impu);
+        LM_ERR("failed to query usrloc for IMPU <%.*s>\n", impu.len, impu.s);
+        return ret;
+    }
+
+    if (res == 0) {
+        if (r->reg_state == IMPU_REGISTERED) ret = 1;
+        ul.unlock_udomain((udomain_t*) _t, &impu);
+        LM_DBG("'%.*s' found in usrloc\n", impu.len, ZSW(impu.s));
+        return ret;
+    }
+
+    ul.unlock_udomain((udomain_t*) _t, &impu);
+    LM_DBG("'%.*s' not found in usrloc\n", impu.len, ZSW(impu.s));
+    return ret;
 }
 
 /**
@@ -269,8 +275,7 @@ int impu_registered(struct sip_msg* _m, char* _t, char* _s)
  * @param _s - s
  * @return true if there is at least one valid contact. false if not
  */
-int term_impu_has_contact(struct sip_msg* _m, udomain_t* _d, char* _s) 
-{
+int term_impu_has_contact(struct sip_msg* _m, udomain_t* _d, char* _s) {
     impurecord_t* r;
     str aor, uri;
     ucontact_t* ptr;
@@ -282,96 +287,89 @@ int term_impu_has_contact(struct sip_msg* _m, udomain_t* _d, char* _s)
     else uri = _m->first_line.u.request.uri;
 
     if (extract_aor(&uri, &aor) < 0) {
-	LM_ERR("failed to extract address of record\n");
-	return -3;
+        LM_ERR("failed to extract address of record\n");
+        return -3;
     }
 
     get_act_time();
 
     ul.lock_udomain(_d, &aor);
     res = ul.get_impurecord(_d, &aor, &r);
-    if (res > 0) {
-	LM_DBG("'%.*s' Not found in usrloc\n", aor.len, ZSW(aor.s));
-	ul.unlock_udomain(_d, &aor);
-	return -1;
+    if (res != 0) {
+        LM_DBG("'%.*s' Not found in usrloc\n", aor.len, ZSW(aor.s));
+        ul.unlock_udomain(_d, &aor);
+        return -1;
     }
 
     while (i < MAX_CONTACTS_PER_IMPU && (ptr = r->newcontacts[i])) {
-	if (VALID_CONTACT(ptr, act_time) && allowed_method(_m, ptr)) {
-	    LM_DBG("Found a valid contact [%.*s]\n", ptr->c.len, ptr->c.s);
-	    i++;
-	    ret = 1;
-	    break;
-	}
-	i++;
+        if (VALID_CONTACT(ptr, act_time) && allowed_method(_m, ptr)) {
+            LM_DBG("Found a valid contact [%.*s]\n", ptr->c.len, ptr->c.s);
+            i++;
+            ret = 1;
+            break;
+        }
+        i++;
     }
 
     /* look first for an un-expired and supported contact */
     if (ptr == 0) {
-	/* nothing found */
-	ret = -1;
-    } 
+        /* nothing found */
+        ret = -1;
+    }
 
     ul.unlock_udomain(_d, &aor);
 
     return ret;
 }
+
 /*! \brief the term_impu_registered() function
  * Return true if the AOR in the Request-URI  for the terminating user is registered
  */
-int term_impu_registered(struct sip_msg* _m, char* _t, char* _s)
-{
-	//str uri, aor;
-	struct sip_msg *req;	
-	int i;
-	str uri;
-	impurecord_t* r;
-	int res;
-
-//	if (_m->new_uri.s) uri = _m->new_uri;
-//	else uri = _m->first_line.u.request.uri;
-//
-//	if (extract_aor(&uri, &aor) < 0) {
-//		LM_ERR("failed to extract address of record\n");
-//		return -1;
-//	}
-	
-	req = _m;	
-	if (!req){
-		LM_ERR(":term_impu_registered: NULL message!!!\n");
-		return -1;
-	}
- 	if (req->first_line.type!=SIP_REQUEST){
- 		req = get_request_from_reply(req);
- 	}
-	
-	if (_m->new_uri.s) uri = _m->new_uri;
-	else uri = _m->first_line.u.request.uri;
-		
-	for(i=0;i<uri.len;i++)
-		if (uri.s[i]==';' || uri.s[i]=='?' || (i>3 /*sip:*/ && uri.s[i]==':' /*strip port*/)) {
-			uri.len = i;
-			break;
-		}
-	LM_DBG("term_impu_registered: Looking for <%.*s>\n",uri.len,uri.s);
-
-	ul.lock_udomain((udomain_t*)_t, &uri);
-	res = ul.get_impurecord((udomain_t*)_t, &uri, &r);
-
-	if (res < 0) {
-		ul.unlock_udomain((udomain_t*)_t, &uri);
-		LM_ERR("failed to query for terminating IMPU <%.*s>\n", uri.len, uri.s);
-		return -1;
-	}
-
-	if (res == 0) {
-		//ul.release_impurecord(r);
-		ul.unlock_udomain((udomain_t*) _t, &uri);
-		LM_DBG("'%.*s' found in usrloc\n", uri.len, ZSW(uri.s));
-		return 1;
-	}
-
-	ul.unlock_udomain((udomain_t*)_t, &uri);
-	LM_DBG("'%.*s' not found in usrloc\n", uri.len, ZSW(uri.s));
-	return -1;
+int term_impu_registered(struct sip_msg* _m, char* _t, char* _s) {
+    struct sip_msg *req;
+    int i;
+    str uri;
+    impurecord_t* r;
+    int res;
+
+    //	if (_m->new_uri.s) uri = _m->new_uri;
+    //	else uri = _m->first_line.u.request.uri;
+    //
+    //	if (extract_aor(&uri, &aor) < 0) {
+    //		LM_ERR("failed to extract address of record\n");
+    //		return -1;
+    //	}
+
+    req = _m;
+    if (!req) {
+        LM_ERR(":term_impu_registered: NULL message!!!\n");
+        return -1;
+    }
+    if (req->first_line.type != SIP_REQUEST) {
+        req = get_request_from_reply(req);
+    }
+
+    if (_m->new_uri.s) uri = _m->new_uri;
+    else uri = _m->first_line.u.request.uri;
+
+    for (i = 0; i < uri.len; i++)
+        if (uri.s[i] == ';' || uri.s[i] == '?' || (i > 3 /*sip:*/ && uri.s[i] == ':' /*strip port*/)) {
+            uri.len = i;
+            break;
+        }
+    LM_DBG("term_impu_registered: Looking for <%.*s>\n", uri.len, uri.s);
+
+    ul.lock_udomain((udomain_t*) _t, &uri);
+    res = ul.get_impurecord((udomain_t*) _t, &uri, &r);
+
+    if (res != 0) {
+        ul.unlock_udomain((udomain_t*) _t, &uri);
+        LM_ERR("failed to query for terminating IMPU or not found <%.*s>\n", uri.len, uri.s);
+        return -1;
+    }
+
+    ul.unlock_udomain((udomain_t*) _t, &uri);
+    LM_DBG("'%.*s' found in usrloc\n", uri.len, ZSW(uri.s));
+    return 1;
+
 }

modules/ims_registrar_scscf/path.c

@@ -40,7 +40,7 @@
  */
 int build_path_vector(struct sip_msg *_m, str *path, str *received)
 {
-	static char buf[MAX_PATH_SIZE];
+	static char buf[MAX_PATH_BUFFER];
 	char *p;
 	struct hdr_field *hdr;
 	struct sip_uri puri;
@@ -59,9 +59,11 @@ int build_path_vector(struct sip_msg *_m, str *path, str *received)
 
 	for( hdr=_m->path,p=buf ; hdr ; hdr = next_sibling_hdr(hdr)) {
 		/* check for max. Path length */
-		if( p-buf+hdr->body.len+1 >= MAX_PATH_SIZE) {
-			LM_ERR("Overall Path body exceeds max. length of %d\n",
-					MAX_PATH_SIZE);
+		if( p-buf+hdr->body.len+1 >= MAX_PATH_BUFFER) {
+			LM_ERR("Overall Path body exceeds max. length of %d - trying to add header [%.*s] and already have [%.*s]\n",
+					MAX_PATH_BUFFER,
+                                        hdr->body.len, hdr->body.s,
+                                        (int)(p-buf), buf);
 			goto error;
 		}
 		if(p!=buf)

modules/ims_registrar_scscf/path.h

@@ -33,6 +33,8 @@
 
 #include "../../parser/msg_parser.h"
 
+#define MAX_PATH_BUFFER 2048
+
 /*! \brief
  * Extracts all Path header bodies into one string and
  * checks if first hop is a loose router. It also extracts

modules/ims_registrar_scscf/regpv.c

@@ -449,7 +449,7 @@ int pv_fetch_contacts(struct sip_msg* msg, char* table, char* uri,
 	ilen = sizeof(ucontact_t);
 	ul.lock_udomain((udomain_t*)table, &aor);
 	res = ul.get_impurecord((udomain_t*)table, &aor, &r);
-	if (res > 0) {
+	if (res != 0) {
 		LM_DBG("'%.*s' Not found in usrloc\n", aor.len, ZSW(aor.s));
 		ul.unlock_udomain((udomain_t*)table, &aor);
 		return -1;

modules/ims_registrar_scscf/save.c

@@ -86,22 +86,19 @@ extern int store_data_on_dereg; /**< should we store SAR user data on de-registr
 extern int ue_unsubscribe_on_dereg;
 extern int user_data_always;
 
-
 /* \brief
  * Return randomized expires between expires-range% and expires.
  * RFC allows only value less or equal to the one provided by UAC.
  */
-static inline int randomize_expires( int expires, int range )
-{
-	/* if no range is given just return expires */
-	if(range == 0) return expires;
+static inline int randomize_expires(int expires, int range) {
+    /* if no range is given just return expires */
+    if (range == 0) return expires;
 
-	int range_min = expires - (float)range/100 * expires;
+    int range_min = expires - (float) range / 100 * expires;
 
-	return range_min + (float)(rand()%100)/100 * ( expires - range_min );
+    return range_min + (float) (rand() % 100) / 100 * (expires - range_min);
 }
 
-
 /*! \brief
  * Calculate absolute expires value per contact as follows:
  * 1) If the contact has expires value, use the value. If it
@@ -157,7 +154,7 @@ static inline int star(udomain_t* _d, str* _a) {
 
     ul.lock_udomain(_d, _a);
 
-    if (ul.delete_impurecord(_d, _a, r) < 0) {
+    if (ul.delete_impurecord(_d, _a, 0) != 0) {
         LM_ERR("failed to remove record from usrloc\n");
 
         /* Delete failed, try to get corresponding
@@ -166,12 +163,12 @@ static inline int star(udomain_t* _d, str* _a) {
          */
         rerrno = R_UL_DEL_R;
 
-        if (!ul.get_impurecord(_d, _a, &r)) {
+        if (ul.get_impurecord(_d, _a, &r) == 0) {
             contact_for_header_t** contact_header = 0;
             build_contact(r, contact_header);
             free_contact_buf(*contact_header);
+            ul.unlock_udomain(_d, _a);
         }
-        ul.unlock_udomain(_d, _a);
         return -1;
     }
     ul.unlock_udomain(_d, _a);
@@ -298,10 +295,10 @@ static inline ucontact_info_t* pack_ci(struct sip_msg* _m, contact_t* _c, unsign
                 }
             }
         }
-	
-	if(_c->params) {
-	    ci.params = _c->params;
-	}
+
+        if (_c->params) {
+            ci.params = _c->params;
+        }
 
         /* set flags */
         ci.flags = _f;
@@ -501,17 +498,15 @@ static inline int is_impu_registered(udomain_t* _d, str* public_identity) {
             LM_DBG("IMPU <%.*s> is not currently registered\n", public_identity->len, public_identity->s);
             ret = 0;
         }
-	
-	//check valid contacts
-        if((impu->num_contacts <=0) || (impu->newcontacts[0]==0))
-	{
-	    LM_DBG("IMPU <%.*s> has no valid contacts\n", public_identity->len, public_identity->s);
+
+        //check valid contacts
+        if ((impu->num_contacts <= 0) || (impu->newcontacts[0] == 0)) {
+            LM_DBG("IMPU <%.*s> has no valid contacts\n", public_identity->len, public_identity->s);
             ret = 0;
-	}
+        }
+        ul.unlock_udomain(_d, public_identity);
     }
-    ul.unlock_udomain(_d, public_identity);
     return ret;
-
 }
 
 struct sip_msg* get_request_from_reply(struct sip_msg *reply) {
@@ -538,6 +533,7 @@ static inline int update_contacts_helper(struct sip_msg* msg, impurecord_t* impu
     qvalue_t qvalue;
     int sos = 0, expires;
     struct ucontact* ucontact;
+    int result;
 
     LM_DBG("updating the contacts for IMPU <%.*s>\n", impu_rec->public_identity.len, impu_rec->public_identity.s);
 
@@ -549,7 +545,6 @@ static inline int update_contacts_helper(struct sip_msg* msg, impurecord_t* impu
 
         case AVP_IMS_SAR_REGISTRATION:
         case AVP_IMS_SAR_RE_REGISTRATION:
-
             for (h = msg->contact; h; h = h->next) {
                 if (h->type == HDR_CONTACT_T && h->parsed) {
 
@@ -571,7 +566,7 @@ static inline int update_contacts_helper(struct sip_msg* msg, impurecord_t* impu
                                 "sos: [%d],"
                                 "expires [%ld]\n", chi->uri.len, chi->uri.s, qvalue, sos, expires - time(NULL));
 
-			LM_DBG("packing contact information\n");
+                        LM_DBG("packing contact information\n");
                         if ((ci = pack_ci(msg, chi, expires, 0)) == 0) {
                             LM_ERR("Failed to extract contact info\n");
                             goto error;
@@ -579,29 +574,31 @@ static inline int update_contacts_helper(struct sip_msg* msg, impurecord_t* impu
 
                         LM_DBG("adding/updating contact based on prior existence\n");
                         //stick the contacts into usrloc
-//			ul.lock_contact_slot(&chi->uri);
-                        if (ul.get_ucontact(impu_rec, &chi->uri, ci->callid,
-                                ci->path, ci->cseq, &ucontact) != 0) {	//get_contact returns with lock
+                        //ul.lock_contact_slot(&chi->uri);
+                        result = ul.get_ucontact(impu_rec, &chi->uri, ci->callid,
+                                ci->path, ci->cseq, &ucontact);
+
+                        if (result != 0) { //get_contact returns with lock
                             LM_DBG("inserting new contact\n");
                             if (ul.insert_ucontact(impu_rec, &chi->uri, ci,
                                     &ucontact) != 0) {
                                 LM_ERR("Error inserting contact <%.*s>\n", chi->uri.len, chi->uri.s);
-//				ul.unlock_contact_slot(&chi->uri);
+                                //				ul.unlock_contact_slot(&chi->uri);
                                 goto error;
                             }
                         } else {
                             LM_DBG("Contact already exists - updating\n");
                             if (ul.update_ucontact(impu_rec, ucontact, ci) != 0) {
                                 LM_ERR("Error updating contact <%.*s>\n", chi->uri.len, chi->uri.s);
-				ul.release_ucontact(ucontact);
-//				ul.unlock_contact_slot(&chi->uri);
+                                ul.release_ucontact(ucontact);
+                                //				ul.unlock_contact_slot(&chi->uri);
                                 goto error;
                             }
-			    ul.release_ucontact(ucontact);
+                            ul.release_ucontact(ucontact);
                         }
-			
-			
-//			ul.unlock_contact_slot(&chi->uri);
+
+
+                        //			ul.unlock_contact_slot(&chi->uri);
                     }
                 }
             }
@@ -614,66 +611,94 @@ error:
     return -1;
 }
 
-/*NB remember to lock udomain pritor to calling this*/
-static inline int unregister_contact(udomain_t* _d, str* public_identity, contact_t* chi) {
+/*NB remember to lock udomain prior to calling this*/
+static inline int unregister_contact(udomain_t* _d, str* public_identity, impurecord_t* _impu_rec, contact_t* chi) {
     impurecord_t* impu_rec;
     struct ucontact* ucontact;
     str callid = {0, 0};
     str path = {0, 0};
-    
+
     reg_subscriber *s;
 
-    if (ul.get_impurecord(_d, public_identity, &impu_rec) != 0) {
-        LM_ERR("Error, no public identity exists for <%.*s>\n", public_identity->len, public_identity->s);
-        goto error;
+    if (_impu_rec) {
+        LM_DBG("already have impurecord....\n");
+        impu_rec = _impu_rec;
+    } else {
+        if (ul.get_impurecord(_d, public_identity, &impu_rec) != 0) {
+            LM_ERR("Error, no public identity exists for <%.*s>\n", public_identity->len, public_identity->s);
+            goto error;
+        }
     }
 
+
+
     if (ul.get_ucontact(impu_rec, &chi->uri, &callid, &path, 0/*cseq*/, &ucontact) != 0) {
         LM_ERR("Can't unregister contact that does not exist <%.*s>\n", chi->uri.len, chi->uri.s);
+        //        ul.unlock_udomain(_d, public_identity);
         goto error;
     }
-    
+
+    get_act_time();
+    if (!VALID_CONTACT(ucontact, act_time)) {
+        LM_DBG("Contact is not valid (expired).... ignoring\n");
+        ul.release_ucontact(ucontact);
+        //        ul.unlock_udomain(_d, public_identity);
+        goto error;
+    }
+
     //Richard added this  - fix to remove subscribes that have presentity and watcher uri same as a contact aor that is being removed
     //When UEs explicitly dereg - they don't unsubscribe, so we remove subscriptions for them
     //only do this if ue_unsubscribe_on_dereg is set to 0
-    if(!ue_unsubscribe_on_dereg){
-	s = impu_rec->shead;
-	LM_DBG("Checking if there is a subscription to this IMPU that has same watcher contact as this contact");
-	while (s) {
-
-	    LM_DBG("Subscription for this impurecord: watcher uri [%.*s] presentity uri [%.*s] watcher contact [%.*s] ", s->watcher_uri.len, s->watcher_uri.s, 
-		    s->presentity_uri.len, s->presentity_uri.s, s->watcher_contact.len, s->watcher_contact.s);
-	    LM_DBG("Contact to be removed [%.*s] ", ucontact->c.len, ucontact->c.s);
-	    if(contact_port_ip_match(&s->watcher_contact, &ucontact->c)) {
-	    //if ((s->watcher_contact.len == ucontact->c.len) && (strncasecmp(s->watcher_contact.s, ucontact->c.s, ucontact->c.len) == 0)) {
-		LM_DBG("This contact has a subscription to its own status - so going to delete the subscription");
-		ul.external_delete_subscriber(s, _d, 0 /*domain is locked*/);
-	    }
-	    s = s->next;
-	}
+    if (!ue_unsubscribe_on_dereg) {
+        s = impu_rec->shead;
+        LM_DBG("Checking if there is a subscription to this IMPU that has same watcher contact as this contact");
+        while (s) {
+
+            LM_DBG("Subscription for this impurecord: watcher uri [%.*s] presentity uri [%.*s] watcher contact [%.*s] ", s->watcher_uri.len, s->watcher_uri.s,
+                    s->presentity_uri.len, s->presentity_uri.s, s->watcher_contact.len, s->watcher_contact.s);
+            LM_DBG("Contact to be removed [%.*s] ", ucontact->c.len, ucontact->c.s);
+            if (contact_port_ip_match(&s->watcher_contact, &ucontact->c)) {
+                //if ((s->watcher_contact.len == ucontact->c.len) && (strncasecmp(s->watcher_contact.s, ucontact->c.s, ucontact->c.len) == 0)) {
+                LM_DBG("This contact has a subscription to its own status - so going to delete the subscription");
+                ul.external_delete_subscriber(s, _d, 0 /*domain is locked*/);
+            }
+            s = s->next;
+        }
     }
-    
-//    if (ul.delete_ucontact(impu_rec, ucontact) != 0) {
-    ul.lock_contact_slot_i(ucontact->contact_hash);
+
+    //    if (ul.delete_ucontact(impu_rec, ucontact) != 0) {
+    ul.lock_contact_slot_i(ucontact->sl);
     if (ul.unlink_contact_from_impu(impu_rec, ucontact, 1) != 0) {
         LM_ERR("Failed to delete ucontact <%.*s>\n", chi->uri.len, chi->uri.s);
     }
-    ul.unlock_contact_slot_i(ucontact->contact_hash);
+    ul.unlock_contact_slot_i(ucontact->sl);
     ul.release_ucontact(ucontact);
     LM_DBG("Contact unlinked successfully <%.*s>\n", chi->uri.len, chi->uri.s);
+    //    ul.unlock_udomain(_d, public_identity);
     return 0;
 
 error:
     return -1;
 }
 
-/* return
- * 1 - success(contacts left) - unregistered contacts and remaining contacts in contact buffer for reply message
- * 2 - success(no contacts left)
- * <=0 - on failure
- * */
+/**
+ * 
+ * @param msg
+ * @param _d
+ * @param public_identity
+ * @param assignment_type
+ * @param s
+ * @param ccf1
+ * @param ccf2
+ * @param ecf1
+ * @param ecf2
+ * @param contact_header
+ * @return  1 - success(contacts left) - unregistered contacts and remaining contacts in contact buffer for reply message
+ *          2 - success(no contacts left)
+ *          <=0 - on failure
+ */
 
-int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
+int update_contacts(struct sip_msg* msg, udomain_t* _d,
         str* public_identity, int assignment_type, ims_subscription** s,
         str* ccf1, str* ccf2, str* ecf1, str* ecf2, contact_for_header_t** contact_header) {
     int reg_state, i, j;
@@ -685,7 +710,7 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
     qvalue_t qvalue;
     int sos = 0;
     ims_subscription* subscription = 0;
-    int first_unbarred_impu = 1;		//this is used to flag the IMPU as anchor for implicit set
+    int first_unbarred_impu = 1; //this is used to flag the IMPU as anchor for implicit set
     int is_primary_impu = 0;
     int ret = 1;
 
@@ -701,17 +726,18 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                 LM_ERR("no userdata supplied for AVP_IMS_SAR_REGISTRATION\n");
                 goto error;
             }
-            for (i = 0; i < (*s)->service_profiles_cnt; i++)
+
+            for (i = 0; i < (*s)->service_profiles_cnt; i++) {
                 for (j = 0; j < (*s)->service_profiles[i].public_identities_cnt; j++) {
                     pi = &((*s)->service_profiles[i].public_identities[j]);
                     ul.lock_udomain(_d, &pi->public_identity);
                     if (first_unbarred_impu && !pi->barring) {
-                    	is_primary_impu = 1;
-                    	first_unbarred_impu = 0;
+                        is_primary_impu = 1;
+                        first_unbarred_impu = 0;
                     } else {
-                    	is_primary_impu = 0;
+                        is_primary_impu = 0;
                     }
-                    if (ul.update_impurecord(_d, &pi->public_identity, reg_state, -1 /*do not change send sar on delete */,
+                    if (ul.update_impurecord(_d, &pi->public_identity, 0, reg_state, -1 /*do not change send sar on delete */,
                             pi->barring, is_primary_impu, s, ccf1, ccf2, ecf1, ecf2, &impu_rec) != 0) {
                         LM_ERR("Unable to update impurecord for <%.*s>\n", pi->public_identity.len, pi->public_identity.s);
                         ul.unlock_udomain(_d, &pi->public_identity);
@@ -726,6 +752,7 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                     }
                     ul.unlock_udomain(_d, &pi->public_identity);
                 }
+            }
             //if we were successful up to this point, then we need to copy the contacts from main impu record (asserted IMPU) into the register response
             ul.lock_udomain(_d, public_identity);
             if (ul.get_impurecord(_d, public_identity, &impu_rec) != 0) {
@@ -762,12 +789,12 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
             if (!subscription) {
                 LM_ERR("No subscriber info associated with <%.*s>, not doing any implicit re-registrations\n", impu_rec->public_identity.len, impu_rec->public_identity.s);
                 //update the new subscription infor for the explicit IMPU
-                if (ul.update_impurecord(_d, public_identity, reg_state, -1 /*do not change send sar on delete */, 0 /*this is explicit so barring must be 0*/, 0, s, ccf1, ccf2,
+                if (ul.update_impurecord(_d, public_identity, 0, reg_state, -1 /*do not change send sar on delete */, 0 /*this is explicit so barring must be 0*/, 0, s, ccf1, ccf2,
                         ecf1, ecf2, &impu_rec) != 0) {
                     LM_ERR("Unable to update explicit impurecord for <%.*s>\n", public_identity->len, public_identity->s);
                 }
-                ul.unlock_udomain(_d, public_identity);
                 build_contact(impu_rec, contact_header);
+                ul.unlock_udomain(_d, public_identity);
                 break;
             }
 
@@ -786,31 +813,20 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                         LM_DBG("Ignoring explicit identity <%.*s>, updating later.....\n", public_identity->len, public_identity->s);
                         continue;
                     }
-                    ul.lock_udomain(_d, &pi->public_identity);
-
-                    //				LM_DBG("implicitly update IMPU <%.*s> for re-registration\n", pi->public_identity.len, pi->public_identity.s);
-                    //				if (ul.get_impurecord(_d, &pi->public_identity,	&tmp_impu_rec) != 0) {
-                    //					LM_ERR("Can't find IMPU for implicit re-registration update.....continuning\n");
-                    //					ul.unlock_udomain(_d, &pi->public_identity);
-                    //					continue;
-                    //				}
 
                     //update the implicit IMPU with the new data
-                    if (ul.update_impurecord(_d, &pi->public_identity,
+                    if (ul.update_impurecord(_d, &pi->public_identity, 0,
                             reg_state, -1 /*do not change send sar on delete */, pi->barring, 0, s, ccf1, ccf2, ecf1, ecf2,
                             &impu_rec) != 0) {
                         LM_ERR("Unable to update implicit impurecord for <%.*s>.... continuing\n", pi->public_identity.len, pi->public_identity.s);
-                        ul.unlock_udomain(_d, &pi->public_identity);
                         continue;
                     }
 
                     //update the contacts for the explicit IMPU
                     if (update_contacts_helper(msg, impu_rec, assignment_type, expires_hdr) != 0) {
                         LM_ERR("Failed trying to update contacts for re-registration of implicit IMPU <%.*s>.......continuing\n", pi->public_identity.len, pi->public_identity.s);
-                        ul.unlock_udomain(_d, &pi->public_identity);
                         continue;
                     }
-                    ul.unlock_udomain(_d, &pi->public_identity);
                 }
             }
             ul.lock_subscription(subscription);
@@ -819,31 +835,34 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
             ul.unlock_subscription(subscription);
 
             //finally we update the explicit IMPU record with the new data
-            ul.lock_udomain(_d, public_identity);
-            if (ul.update_impurecord(_d, public_identity, reg_state, -1 /*do not change send sar on delete */, 0 /*this is explicit so barring must be 0*/, 0, s, ccf1, ccf2, ecf1, ecf2, &impu_rec) != 0) {
+            if (ul.update_impurecord(_d, public_identity, 0, reg_state, -1 /*do not change send sar on delete */, 0 /*this is explicit so barring must be 0*/, 0, s, ccf1, ccf2, ecf1, ecf2, &impu_rec) != 0) {
                 LM_ERR("Unable to update explicit impurecord for <%.*s>\n", public_identity->len, public_identity->s);
             }
-            ul.unlock_udomain(_d, public_identity);
             break;
         case AVP_IMS_SAR_USER_DEREGISTRATION:
             /*TODO: if its not a star lets find all the contact records and remove them*/
             ul.lock_udomain(_d, public_identity);
+
+            if (ul.get_impurecord(_d, public_identity, &impu_rec) != 0) {
+                LM_ERR("Error retrieving impu record\n");
+                ul.unlock_udomain(_d, public_identity);
+                goto error;
+            }
+
             for (h = msg->contact; h; h = h->next) {
                 if (h->type == HDR_CONTACT_T && h->parsed) {
                     for (chi = ((contact_body_t*) h->parsed)->contacts; chi; chi = chi->next) {
                         if (calc_contact_q(chi->q, &qvalue) != 0) {
                             LM_ERR("error on <%.*s>\n", chi->uri.len, chi->uri.s);
-                            ul.unlock_udomain(_d, public_identity);
                             goto error;
                         }
                         sos = cscf_get_sos_uri_param(chi->uri);
                         if (sos < 0) {
                             LM_ERR("Error trying to determine if this is a sos contact <%.*s>\n", chi->uri.len, chi->uri.s);
-                            ul.unlock_udomain(_d, public_identity);
                             goto error;
                         }
                         calc_contact_expires(chi, expires_hdr, sos);
-                        if (unregister_contact(_d, public_identity, chi) != 0) {
+                        if (unregister_contact(_d, public_identity, impu_rec, chi) != 0) {
                             LM_ERR("Unable to remove contact <%.*s\n", chi->uri.len, chi->uri.s);
 
                         }
@@ -852,14 +871,8 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                     }
                 }
             }
-            /*now lets see if we still have any contacts left to decide on return value*/
-            if (ul.get_impurecord(_d, public_identity, &impu_rec) != 0) {
-                LM_ERR("Error retrieving impu record\n");
-                ul.unlock_udomain(_d, public_identity);
-                goto error;
-            }
-
-            if (impu_rec->num_contacts>=0 && impu_rec->newcontacts[0]) {
+ 
+            if (impu_rec->num_contacts >= 0 && impu_rec->newcontacts[0]) {
                 LM_DBG("contacts still available\n");
                 //TODO: add all other remaining contacts to reply message (contacts still registered for this IMPU)
                 ret = 1;
@@ -879,7 +892,7 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                 ul.unlock_subscription(subscription);
 
                 ul.unlock_udomain(_d, public_identity);
-
+                //TODO: this needs a clean......
                 //lock(subscription->lock);
                 for (i = 0; i < subscription->service_profiles_cnt; i++) {
                     for (j = 0; j < subscription->service_profiles[i].public_identities_cnt; j++) {
@@ -911,7 +924,7 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                                         goto error;
                                     }
                                     calc_contact_expires(chi, expires_hdr, sos);
-                                    if (unregister_contact(_d, &pi->public_identity, chi) != 0) {
+                                    if (unregister_contact(_d, 0, tmp_impu_rec, chi) != 0) {
                                         LM_ERR("Unable to remove contact <%.*s\n", chi->uri.len, chi->uri.s);
 
                                     }
@@ -919,21 +932,16 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                             }
                         }
                         /*now lets see if we still have any contacts left to decide on return value*/
-                        if (ul.get_impurecord(_d, &pi->public_identity, &impu_rec) != 0) {
-                            LM_ERR("Error retrieving impu record for implicit de-reg....continuing\n");
-                            ul.unlock_udomain(_d, &pi->public_identity);
-                            continue;
-                        }
 
-                        if (impu_rec->num_contacts && impu_rec->newcontacts[0])
+                        if (tmp_impu_rec->num_contacts && tmp_impu_rec->newcontacts[0])
                             LM_DBG("contacts still available after implicit dereg for IMPU: <%.*s>\n", pi->public_identity.len, pi->public_identity.s);
                         else {
                             LM_DBG("no contacts left after implicit dereg for IMPU: <%.*s>\n", pi->public_identity.len, pi->public_identity.s);
                             LM_DBG("Updating impu record to not send SAR on delete as this is explicit dereg");
-			    reg_state = IMS_USER_REGISTERED;//keep reg_state as it is
-			    if (ul.update_impurecord(_d, &pi->public_identity, reg_state, 0 /*do not send sar on delete */, -1 /*do not change barring*/, 0, 0, 0, 0, 0, 0, &tmp_impu_rec) != 0) {
-				    LM_ERR("Unable to update explicit impurecord for <%.*s>\n", pi->public_identity.len, pi->public_identity.s);
-			    }
+                            reg_state = IMS_USER_REGISTERED; //keep reg_state as it is
+                            if (ul.update_impurecord(_d, 0/*&pi->public_identity*/, tmp_impu_rec, reg_state, 0 /*do not send sar on delete */, -1 /*do not change barring*/, 0, 0, 0, 0, 0, 0, &tmp_impu_rec) != 0) {
+                                LM_ERR("Unable to update explicit impurecord for <%.*s>\n", pi->public_identity.len, pi->public_identity.s);
+                            }
                         }
 
                         ul.unlock_udomain(_d, &pi->public_identity);
@@ -944,15 +952,16 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                 ul.unlock_subscription(subscription);
             }
 
-	    if (ret == 2) {
+            //TODO: clean here too - maybe do earlier with the lock on the domain already held...
+            if (ret == 2) {
                 LM_DBG("no contacts left after explicit dereg for IMPU: <%.*s>\n", public_identity->len, public_identity->s);
                 LM_DBG("Updating impu record to not send SAR on delete as this is explicit dereg");
-		reg_state = IMS_USER_REGISTERED;//keep reg_state as it is
-		ul.lock_udomain(_d, public_identity);
-		if (ul.update_impurecord(_d, public_identity, reg_state, 0 /*do not send sar on delete */, -1 /*do not change barring*/, 0, 0, 0, 0, 0, 0, &tmp_impu_rec) != 0) {
-			LM_ERR("Unable to update explicit impurecord for <%.*s>\n", public_identity->len, public_identity->s);
-		}
-		ul.unlock_udomain(_d, public_identity);
+                reg_state = IMS_USER_REGISTERED; //keep reg_state as it is
+                ul.lock_udomain(_d, public_identity);
+                if (ul.update_impurecord(_d, public_identity, 0, reg_state, 0 /*do not send sar on delete */, -1 /*do not change barring*/, 0, 0, 0, 0, 0, 0, &tmp_impu_rec) != 0) {
+                    LM_ERR("Unable to update explicit impurecord for <%.*s>\n", public_identity->len, public_identity->s);
+                }
+                ul.unlock_udomain(_d, public_identity);
             }
             break;
 
@@ -964,7 +973,7 @@ int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
                         j++) {
                     pi = &((*s)->service_profiles[i].public_identities[j]);
                     ul.lock_udomain(_d, &pi->public_identity);
-                    if (ul.update_impurecord(_d, &pi->public_identity, reg_state, -1 /*do not change send sar on delete */,
+                    if (ul.update_impurecord(_d, &pi->public_identity, 0, reg_state, -1 /*do not change send sar on delete */,
                             pi->barring, 0, s, ccf1, ccf2, ecf1, ecf2, &impu_rec)
                             != 0) {
                         LM_ERR("Unable to update impurecord for <%.*s>\n", pi->public_identity.len, pi->public_identity.s);
@@ -1228,7 +1237,7 @@ int save(struct sip_msg* msg, char* str1, char *route) {
             //unregister the requested contacts, if none left at the end then send a SAR, otherwise return successfully
             LM_DBG("need to unregister contacts\n");
             //lets update the contacts - we need to know if all were deleted or not for the public identity
-            int res = update_contacts_new(msg, _d, &public_identity, sar_assignment_type, 0, 0, 0, 0, 0, &contact_header);
+            int res = update_contacts(msg, _d, &public_identity, sar_assignment_type, 0, 0, 0, 0, 0, &contact_header);
             if (res <= 0) {
                 LM_ERR("Error processing REGISTER for de-registration\n");
                 free_contact_buf(contact_header);
@@ -1288,7 +1297,7 @@ int save(struct sip_msg* msg, char* str1, char *route) {
     saved_t->expires = expires;
     saved_t->require_user_data = require_user_data;
     saved_t->sar_assignment_type = sar_assignment_type;
-    
+
     saved_t->domain = _d;
 
     saved_t->public_identity.s = (char*) shm_malloc(public_identity.len + 1);
@@ -1329,7 +1338,7 @@ no_sar:
 
     update_stat(accepted_registrations, 1);
 
-        //we must send the de reged contacts in this! so we only free the contact header after sending
+    //we must send the de reged contacts in this! so we only free the contact header after sending
     /* Only send reply upon request, not upon reply */
     if ((is_route_type(REQUEST_ROUTE)) && (reg_send_reply(msg, contact_header) < 0)) {
         free_contact_buf(contact_header);

modules/ims_registrar_scscf/save.h

@@ -62,7 +62,7 @@ int save(struct sip_msg* msg, char* str1, char* route);
 
 int unregister(struct sip_msg* _m, char* _d, char* _uri);
 
-int update_contacts_new(struct sip_msg* msg, udomain_t* _d,
+int update_contacts(struct sip_msg* msg, udomain_t* _d,
         str* public_identity, int assignment_type, ims_subscription** s,
         str* ccf1, str* ccf2, str* ecf1, str* ecf2, contact_for_header_t** contact_header);
 

modules/ims_registrar_scscf/userdata_parser.c

@@ -872,6 +872,8 @@ static ims_subscription* parse_ims_subscription(xmlDocPtr doc, xmlNodePtr root)
 		shm_free(s);
 		return 0;
 	}
+        
+        s->sl = -1; //this tells us the subscription is not linked to a list....
 #ifdef EXTRA_DEBUG
     	LM_DBG("LOCK CREATED FOR SUBSCRIPTION [%.*s]: %p\n", s->private_identity.len, s->private_identity.s, s->lock);
 #endif

modules/ims_registrar_scscf/usrloc_cb.c

@@ -87,7 +87,7 @@ void ul_contact_changed(impurecord_t* r, ucontact_t* c, int type, void* param) {
 
     LM_DBG("Received notification of type %d on contact Address <%.*s>", type, c->c.len, c->c.s);
     
-    if(!r->shead){
+    if(!r->shead) {
         LM_DBG("There are no subscriptions for this IMPU therefore breaking out now as nothing to do");
         return;
     }

modules/ims_usrloc_scscf/hslot.c

@@ -204,19 +204,20 @@ void slot_add(hslot_t* _s, struct impurecord* _r)
  */
 void slot_rem(hslot_t* _s, struct impurecord* _r)
 {
-	if (_r->prev) {
-		_r->prev->next = _r->next;
-	} else {
-		_s->first = _r->next;
-	}
-
-	if (_r->next) {
-		_r->next->prev = _r->prev;
-	} else {
-		_s->last = _r->prev;
-	}
-
-	_r->prev = _r->next = 0;
-	_r->slot = 0;
-	_s->n--;
+    LM_DBG("Removing IMPU [%.*s] from hashtable\n", _r->public_identity.len, _r->public_identity.s);
+    if (_r->prev) {
+        _r->prev->next = _r->next;
+    } else {
+        _s->first = _r->next;
+    }
+
+    if (_r->next) {
+        _r->next->prev = _r->prev;
+    } else {
+        _s->last = _r->prev;
+    }
+
+    _r->prev = _r->next = 0;
+    _r->slot = 0;
+    _s->n--;
 }

modules/ims_usrloc_scscf/hslot.h

@@ -66,6 +66,9 @@ typedef struct hslot {
 #else
 	int lockidx;            /*!< Lock index for hash entry - the rest*/
 #endif
+        atomic_t locker_pid;
+        int recursive_lock_level;
+       
 } hslot_t;
 
 /*! \brief

modules/ims_usrloc_scscf/hslot_sp.c

@@ -44,6 +44,7 @@
  */
 
 #include "hslot_sp.h"
+#include "ul_scscf_stats.h"
 
 /*! number of locks */
 int subs_locks_no=4;
@@ -191,6 +192,7 @@ void subs_slot_add(hslot_sp_t* _s, struct ims_subscription_s* _r)
 		_s->last = _r;
 	}
 	_s->n++;
+        counter_inc(ul_scscf_cnts_h.active_subscriptions);
 	_r->slot = _s;
 }
 
@@ -217,29 +219,9 @@ void subs_slot_rem(hslot_sp_t* _s, struct ims_subscription_s* _r)
 	_r->prev = _r->next = 0;
 	_r->slot = 0;
 	_s->n--;
-}
-
-void print_subscription(ims_subscription* s) {
-    LM_DBG("IMS Subscription: [%.*s], ref: [%d]\n", s->private_identity.len, s->private_identity.s, s->ref_count);
-}
-
-int sync_subscriptions() {
-    int i, count=0;
-    ims_subscription* ims_subscription;
-    
-    for (i = 0; i < ims_subscription_list->size - 1; i++) {
-        lock_get(ims_subscription_list->slot[i].lock);
-        count += ims_subscription_list->slot[i].n;
-        ims_subscription = ims_subscription_list->slot[i].first;
-        while (ims_subscription) {
-            //do stuff (check ref for example)
-            print_subscription(ims_subscription);
-            ims_subscription = ims_subscription->next;
+        counter_add(ul_scscf_cnts_h.active_subscriptions, -1);
+        if (_s->n < 0) {
+            LM_WARN("we should not go negative....\n");
+            _s->n = 0;
         }
-        lock_release(ims_subscription_list->slot[i].lock);
-    }
-
-    LM_DBG("Active Subscriptions: [%d]\n", count);
-    
-    return 0;
 }

modules/ims_usrloc_scscf/hslot_sp.h

@@ -103,7 +103,4 @@ void subs__lock_idx(int idx);
 void subs__release_idx(int idx);
 #endif
 
-int sync_subscriptions();
-void print_subscription(ims_subscription* s);
-
 #endif /* HSLOTSP_H */

modules/ims_usrloc_scscf/impurecord.c

@@ -74,7 +74,6 @@ extern int unreg_validity;
 extern int maxcontact_behaviour;
 extern int maxcontact;
 extern int db_mode;
-extern struct ul_scscf_counters_h ul_scscf_cnts_h;
 
 extern int sub_dialog_hash_size;
 extern int subs_hash_size;
@@ -84,6 +83,9 @@ extern struct ims_subscription_list* ims_subscription_list;
 
 extern struct dlg_binds dlgb;
 
+static ucontact_t* contacts_to_expire [MAX_CONTACTS_PER_IMPU]; //this is done to prevent fragmentation of memory...
+static int num_contacts_to_expire;
+
 /*!
  * \brief Create and initialize new record structure
  * \param _dom domain name
@@ -91,7 +93,7 @@ extern struct dlg_binds dlgb;
  * \param _r pointer to the new record
  * \return 0 on success, negative on failure
  */
-int new_impurecord(str* _dom, str* public_identity, int reg_state, int barring, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, impurecord_t** _r) {
+int new_impurecord(str* _dom, str* public_identity, str* private_identity, int reg_state, int barring, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, impurecord_t** _r) {
     *_r = (impurecord_t*) shm_malloc(sizeof (impurecord_t));
     if (*_r == 0) {
         LM_ERR("no more shared memory\n");
@@ -105,8 +107,7 @@ int new_impurecord(str* _dom, str* public_identity, int reg_state, int barring,
     if ((*_r)->cbs == 0) {
         LM_CRIT("no more shared mem\n");
         shm_free(*_r);
-        *_r = 0;
-        return -2;
+        goto error;
     }
     (*_r)->cbs->first = 0;
     (*_r)->cbs->reg_types = 0;
@@ -115,11 +116,20 @@ int new_impurecord(str* _dom, str* public_identity, int reg_state, int barring,
     if ((*_r)->public_identity.s == 0) {
         LM_ERR("no more shared memory\n");
         shm_free(*_r);
-        *_r = 0;
-        return -2;
+        goto error;
     }
     memcpy((*_r)->public_identity.s, public_identity->s, public_identity->len);
     (*_r)->public_identity.len = public_identity->len;
+
+    (*_r)->private_identity.s = (char*) shm_malloc(private_identity->len);
+    if ((*_r)->private_identity.s == 0) {
+        LM_ERR("no more shared memory\n");
+        shm_free(*_r);
+        goto error;
+    }
+    memcpy((*_r)->private_identity.s, private_identity->s, private_identity->len);
+    (*_r)->private_identity.len = private_identity->len;
+
     (*_r)->domain = _dom;
     (*_r)->aorhash = core_hash(public_identity, 0, 0);
     (*_r)->reg_state = reg_state;
@@ -133,15 +143,20 @@ int new_impurecord(str* _dom, str* public_identity, int reg_state, int barring,
     if (ecf2 && ecf2->len > 0) STR_SHM_DUP((*_r)->ecf2, *ecf2, "ECF2");
     /*assign ims subscription profile*/
     if (s && *s) {
-         ref_subscription_unsafe(*s);
-	(*_r)->s = *s;
+        ref_subscription_unsafe(*s);
+        (*_r)->s = *s;
     }
 
     return 0;
 
 out_of_memory:
     LM_ERR("no more shared memory\n");
+    *_r = 0;
     return -3;
+error:
+    LM_ERR("Failed to create new impurecord...\n");
+    *_r = 0;
+    return -2;
 }
 
 /*!
@@ -167,7 +182,7 @@ void free_impurecord(impurecord_t* _r) {
     if (_r->ecf2.s)
         shm_free(_r->ecf2.s);
     if (_r->s) {
-        unref_subscription_unsafe(_r->s);
+        unref_subscription(_r->s);
     }
 
     /*remove REG subscriptions to this IMPU*/
@@ -177,10 +192,14 @@ void free_impurecord(impurecord_t* _r) {
         free_subscriber(subscriber);
         subscriber = s_tmp;
     }
+    _r->shead = 0;
 
     if (_r->public_identity.s)
         shm_free(_r->public_identity.s);
 
+    if (_r->private_identity.s)
+        shm_free(_r->private_identity.s);
+
     //free callback list
     for (cbp = _r->cbs->first; cbp;) {
         cbp_tmp = cbp;
@@ -190,8 +209,6 @@ void free_impurecord(impurecord_t* _r) {
         shm_free(cbp_tmp);
     }
     shm_free(_r->cbs);
-
-
     shm_free(_r);
 }
 
@@ -203,7 +220,7 @@ void free_impurecord(impurecord_t* _r) {
 void print_impurecord(FILE* _f, impurecord_t* _r) {
     ucontact_t* ptr;
     int i;
-    
+
     fprintf(_f, "...Record(%p)...\n", _r);
     fprintf(_f, "domain : '%.*s'\n", _r->domain->len, ZSW(_r->domain->s));
     fprintf(_f, "public_identity    : '%.*s'\n", _r->public_identity.len, ZSW(_r->public_identity.s));
@@ -233,7 +250,7 @@ void print_impurecord(FILE* _f, impurecord_t* _r) {
     }
 
     if (_r->newcontacts[0]) {
-        while ((ptr=_r->newcontacts[i++])) {
+        while ((ptr = _r->newcontacts[i++])) {
             print_ucontact(_f, ptr);
         }
     }
@@ -254,16 +271,16 @@ void print_impurecord(FILE* _f, impurecord_t* _r) {
 ucontact_t* mem_insert_ucontact(impurecord_t* _r, str* _c, ucontact_info_t* _ci) {
     ucontact_t* c;
     int sl;
-    
+
     if ((c = new_ucontact(_r->domain, &_r->public_identity, _c, _ci)) == 0) {
         LM_ERR("failed to create new contact\n");
         return 0;
     }
     counter_inc(ul_scscf_cnts_h.active_contacts);
 
-    LM_DBG("Created new contact in memory with AOR: [%.*s] and hash [%d]\n", _c->len, _c->s, c->contact_hash);
-    
-    sl = (c->contact_hash);// & (contact_list->size - 1);
+    LM_DBG("Created new contact in memory with AOR: [%.*s] and hash [%d]\n", _c->len, _c->s, c->sl);
+
+    sl = (c->sl);
     lock_contact_slot_i(sl);
     contact_slot_add(&contact_list->slot[sl], c);
     unlock_contact_slot_i(sl);
@@ -277,8 +294,9 @@ ucontact_t* mem_insert_ucontact(impurecord_t* _r, str* _c, ucontact_info_t* _ci)
  * \param _c removed contact
  */
 void mem_remove_ucontact(ucontact_t* _c) {
-    LM_DBG("removing contact [%.*s] from slot %d\n", _c->c.len, _c->c.s, _c->contact_hash);
-    contact_slot_rem(&contact_list->slot[_c->contact_hash], _c);
+    LM_DBG("removing contact [%.*s] from slot %d\n", _c->c.len, _c->c.s, _c->sl);
+    contact_slot_rem(&contact_list->slot[_c->sl], _c);
+    counter_add(ul_scscf_cnts_h.active_contacts, -1);
 }
 
 /*!
@@ -287,17 +305,15 @@ void mem_remove_ucontact(ucontact_t* _c) {
  * \param _c deleted contact
  */
 void mem_delete_ucontact(ucontact_t* _c) {
-    
+
     struct contact_dialog_data *dialog_data;
     //tear down dialogs in dialog data list
     for (dialog_data = _c->first_dialog_data; dialog_data;) {
-        dlgb.lookup_terminate_dlg(dialog_data->h_entry, dialog_data->h_id, NULL );
+        dlgb.lookup_terminate_dlg(dialog_data->h_entry, dialog_data->h_id, NULL);
         dialog_data = dialog_data->next;
     }
-    
+
     mem_remove_ucontact(_c);
-    //TODO: fix stats
-//    if_update_stat(_r->slot, _r->slot->d->contacts, -1);
     free_ucontact(_c);
 }
 
@@ -308,18 +324,17 @@ void mem_delete_ucontact(ucontact_t* _c) {
  * the record, delete the expired ones from memory.
  * \param _r processed record
  */
-static inline void nodb_timer(impurecord_t* _r) {
+static inline void process_impurecord(impurecord_t* _r) {
+    int flag, mustdeleteimpu = 1, n, k;
+    unsigned int sl;
     ucontact_t* ptr;
-    int i, flag, mustdeleteimpu=1, hascontacts=0;
-    udomain_t* udomain;
-    unsigned int hash_code = 0;
-    impurecord_t* tmp_impu;
-
+    int hascontacts;
+    udomain_t* _d;
     reg_subscriber *s;
     subs_t* sub_dialog;
 
     get_act_time();
-    
+
     s = _r->shead;
     LM_DBG("Checking validity of IMPU: <%.*s> registration subscriptions\n", _r->public_identity.len, _r->public_identity.s);
     while (s) {
@@ -328,88 +343,93 @@ static inline void nodb_timer(impurecord_t* _r) {
                     s->watcher_contact.len, s->watcher_contact.s, s->presentity_uri.len, s->presentity_uri.s);
             delete_subscriber(_r, s);
         } else {
-	    mustdeleteimpu = 0;
             LM_DBG("DBG:registrar_timer: Subscriber with watcher_contact <%.*s> and presentity uri <%.*s> is valid and expires in %d seconds.\n",
                     s->watcher_contact.len, s->watcher_contact.s, s->presentity_uri.len, s->presentity_uri.s,
                     (unsigned int) (s->expires - time(NULL)));
-	    hash_code = core_hash(&s->call_id, &s->to_tag, sub_dialog_hash_size);
-	    LM_DBG("Hash size: <%i>", sub_dialog_hash_size);
-	    LM_DBG("Searching sub dialog hash info with call_id: <%.*s> and ttag <%.*s> ftag <%.*s> and hash code <%i>", s->call_id.len, s->call_id.s, s->to_tag.len, s->to_tag.s, s->from_tag.len, s->from_tag.s, hash_code);
-	    /* search the record in hash table */
-	    lock_get(&sub_dialog_table[hash_code].lock);
-	    sub_dialog= pres_search_shtable(sub_dialog_table, s->call_id, s->to_tag, s->from_tag, hash_code);
-	    if(sub_dialog== NULL)
-	    {
-		LM_ERR("DBG:registrar_timer: Subscription has no dialog record in hash table\n");
-	    }else {
-		LM_DBG("DBG:registrar_timer: Subscription has dialog record in hash table with presentity uri <%.*s>\n", sub_dialog->pres_uri.len, sub_dialog->pres_uri.s);
-	    }
-	    
-	    lock_release(&sub_dialog_table[hash_code].lock);
+            sl = core_hash(&s->call_id, &s->to_tag, sub_dialog_hash_size);
+            LM_DBG("Hash size: <%i>", sub_dialog_hash_size);
+            LM_DBG("Searching sub dialog hash info with call_id: <%.*s> and ttag <%.*s> ftag <%.*s> and hash code <%i>", s->call_id.len, s->call_id.s, s->to_tag.len, s->to_tag.s, s->from_tag.len, s->from_tag.s, sl);
+            /* search the record in hash table */
+            lock_get(&sub_dialog_table[sl].lock);
+            sub_dialog = pres_search_shtable(sub_dialog_table, s->call_id, s->to_tag, s->from_tag, sl);
+            if (sub_dialog == NULL) {
+                LM_ERR("DBG:registrar_timer: Subscription has no dialog record in hash table\n");
+            } else {
+                LM_DBG("DBG:registrar_timer: Subscription has dialog record in hash table with presentity uri <%.*s>\n", sub_dialog->pres_uri.len, sub_dialog->pres_uri.s);
+            }
+            lock_release(&sub_dialog_table[sl].lock);
         }
         s = s->next;
     }
 
-    LM_DBG("Checking validity of IMPU: <%.*s> contacts (#%d contacts)\n", _r->public_identity.len, _r->public_identity.s, _r->num_contacts);
+    LM_DBG("\tPublic Identity %.*s, Barred: [%d], State: [%s]\n",
+            _r->public_identity.len, _r->public_identity.s,
+            _r->barring,
+            get_impu_regstate_as_string(_r->reg_state));
     flag = 0;
-    
-    for (i=0; i<MAX_CONTACTS_PER_IMPU; i++) {
-	if ((ptr = _r->newcontacts[i])) {
-	    flag=1;
-	    if (!VALID_CONTACT(ptr, act_time)) {
-		LM_DBG("IMPU:<%.*s> - contact:<%.*s> has expired\n", _r->public_identity.len, _r->public_identity.s, ptr->c.len, ptr->c.s);
-		lock_contact_slot_i(ptr->contact_hash); //TODO, this maybe should be a lock on the strcuture itself and not the slot
-		unlink_contact_from_impu(_r, ptr, 1);
-		unlock_contact_slot_i(ptr->contact_hash);
-	    } else {
-		LM_DBG("IMPU:<%.*s> - contact:<%.*s> is valid and expires in %d seconds\n", _r->public_identity.len, _r->public_identity.s,
-                    ptr->c.len, ptr->c.s,
-                    (unsigned int) (ptr->expires - time(NULL)));
-		mustdeleteimpu = 0;
-		hascontacts = 1;
-	    }
-	    
-	} else {
-	    break;
-	}
+    mustdeleteimpu = 1;
+    hascontacts = 0;
+    num_contacts_to_expire = 0;
+    for (k = 0; (k < _r->num_contacts) && (k < MAX_CONTACTS_PER_IMPU); k++) {
+        if ((ptr = _r->newcontacts[k])) {
+            flag = 1;
+            if (!VALID_CONTACT(ptr, act_time)) {
+                LM_DBG("IMPU:<%.*s> - contact:<%.*s> has expired... unlinking contact from IMPU\n", _r->public_identity.len, _r->public_identity.s, ptr->c.len, ptr->c.s);
+                contacts_to_expire[num_contacts_to_expire] = ptr;
+                num_contacts_to_expire++;
+            } else {
+                LM_DBG("\t\tContact #%i - %.*s, Ref [%d] (expires in %ld seconds)\n", k, ptr->c.len, ptr->c.s, ptr->ref_count, ptr->expires - act_time);
+                mustdeleteimpu = 0;
+                hascontacts = 1;
+            }
+        } else {
+            LM_WARN("num_contacts and actual data not consistent... .aborting\n");
+            break;
+        }
     }
-    
+
+    if (num_contacts_to_expire > 0) {
+        LM_DBG("\tThere are %d contacts to expire\n", num_contacts_to_expire);
+        for (n = 0; n < num_contacts_to_expire; n++) {
+            ptr = contacts_to_expire[n];
+            LM_DBG("\t\texpiring contact %i: [%.*s] in slot [%d]\n", n, contacts_to_expire[n]->c.len, contacts_to_expire[n]->c.s, contacts_to_expire[n]->sl);
+            sl = ptr->sl;
+            lock_contact_slot_i(sl);
+            unlink_contact_from_impu(_r, ptr, 1);
+            unlock_contact_slot_i(sl);
+        }
+    }
+
     if (!flag)
         LM_DBG("no contacts\n");
 
-    register_udomain("location", &udomain);
     if (mustdeleteimpu) {
-	delete_impurecord(udomain, &_r->public_identity, _r);
+        register_udomain("location", &_d);
+        delete_impurecord(_d, &_r->public_identity, _r);
     } else {
-	if (!hascontacts) {
-	    LM_DBG("This impu is not to be deleted but has no contacts - should change state to IMPU_UNREGISTERED\n");
-	    if (update_impurecord(udomain, &_r->public_identity, IMPU_UNREGISTERED,
-		    -1/*do not change*/, -1 /*do not change */, -1/*do not change*/, NULL, NULL, NULL, NULL, NULL, &tmp_impu) != 0) {
-		LM_ERR("Unable to update impurecord for <%.*s>\n", _r->public_identity.len, _r->public_identity.s);
-	    }
-	}
+        if (!hascontacts) {
+            LM_DBG("This impu is not to be deleted but has no contacts - changing state to IMPU_UNREGISTERED\n");
+            _r->reg_state = IMPU_UNREGISTERED;
+        }
     }
 }
 
 /*!
- * \brief Run timer functions depending on the db_mode setting.
- *
- * Helper function that run the appropriate timer function, depending
- * on the db_mode setting.
- * \param _r processed record
+ * \brief Process impurecords (check contacts for expiry, etc (assume domain slot is locked)
+ * @param _r impurecord to process
  */
 void timer_impurecord(impurecord_t* _r) {
-        nodb_timer(_r);
+    process_impurecord(_r);
 }
 
 int get_contacts_count(impurecord_t* _r) {
     ucontact_t* ptr;
     int i = 0;
-    
-    while (i<MAX_CONTACTS_PER_IMPU && (ptr=_r->newcontacts[i])) {
-	i++;
+
+    while (i < MAX_CONTACTS_PER_IMPU && (ptr = _r->newcontacts[i])) {
+        i++;
     }
-    
+
     return i;
 }
 
@@ -449,15 +469,13 @@ int insert_ucontact(impurecord_t* _r, str* _contact, ucontact_info_t* _ci, ucont
         return -1;
     }
 
+    //    /*DB?*/
+    if (db_mode == WRITE_THROUGH && db_insert_ucontact(_r, *_c) != 0) {
+        LM_ERR("error inserting contact into db");
+        return -1;
+    }
 
-
-//    /*DB?*/
-	if (db_mode == WRITE_THROUGH && db_insert_ucontact(_r, *_c) != 0) {
-		LM_ERR("error inserting contact into db");
-		return -1;
-	}
-
-//make sure IMPU is linked to this contact
+    //make sure IMPU is linked to this contact
     link_contact_to_impu(_r, *_c, 1);
     
     release_ucontact(*_c);
@@ -471,7 +489,6 @@ int insert_ucontact(impurecord_t* _r, str* _contact, ucontact_info_t* _ci, ucont
 
     return 0;
 }
-
 /*!
  * \brief Delete ucontact from impurecord
  * \param _r record where the contact belongs to
@@ -480,56 +497,43 @@ int insert_ucontact(impurecord_t* _r, str* _contact, ucontact_info_t* _ci, ucont
  */
 int delete_ucontact(struct ucontact* _c) {
     int ret = 0;
-    
-    LM_DBG("Deleting contact: [%.*s]\n", _c->c.len, _c->c.s);
-    //TODO: restore callbacks
-//    if (exists_ulcb_type(_c->cbs, UL_CONTACT_DELETE)) {
-//        run_ul_callbacks(_c->cbs, UL_CONTACT_DELETE, _r, _c);
-//    }
-    
-    //TODO: following callbacks need to move to unlink contact from impu functions
-//    if (exists_ulcb_type(_r->cbs, UL_IMPU_DELETE_CONTACT)) {
-//        run_ul_callbacks(_r->cbs, UL_IMPU_DELETE_CONTACT, _r, _c);
-//    }
-
-	/*DB?*/
-	if (db_mode == WRITE_THROUGH && db_delete_ucontact(_c) != 0) {
-		LM_ERR("error removing contact from DB [%.*s]... will still remove from memory\n", _c->c.len, _c->c.s);
-
-	}
 
+    LM_DBG("Deleting contact: [%.*s]\n", _c->c.len, _c->c.s);
+    /*DB?*/
+    if (db_mode == WRITE_THROUGH && db_delete_ucontact(_c) != 0) {
+        LM_ERR("error removing contact from DB [%.*s]... will still remove from memory\n", _c->c.len, _c->c.s);
+    }
     mem_delete_ucontact(_c);
 
     return ret;
 }
 
-
 /* function to convert contact aor to only have data after @ - ie strip user part */
 inline int aor_to_contact(str* aor, str* contact) {
-	char* p;
-	int ret = 0;	//success
-
-	contact->s = aor->s;
-	contact->len = aor->len;
-	if (memcmp(aor->s, "sip:", 4) == 0) {
-		contact->s = aor->s + 4;
-		contact->len-=4;
-	}
-
-	if ((p=memchr(contact->s, '@', contact->len))) {
-		contact->len -= (p - contact->s + 1);
-		contact->s = p+1;
-	}
-
-	if ((p=memchr(contact->s, ';', contact->len))) {
-		contact->len = p - contact->s;
-	}
-
-	if ((p=memchr(contact->s, '>', contact->len))) {
-		contact->len = p - contact->s;
-	}
-
-	return ret;
+    char* p;
+    int ret = 0; //success
+
+    contact->s = aor->s;
+    contact->len = aor->len;
+    if (memcmp(aor->s, "sip:", 4) == 0) {
+        contact->s = aor->s + 4;
+        contact->len -= 4;
+    }
+
+    if ((p = memchr(contact->s, '@', contact->len))) {
+        contact->len -= (p - contact->s + 1);
+        contact->s = p + 1;
+    }
+
+    if ((p = memchr(contact->s, ';', contact->len))) {
+        contact->len = p - contact->s;
+    }
+
+    if ((p = memchr(contact->s, '>', contact->len))) {
+        contact->len = p - contact->s;
+    }
+
+    return ret;
 }
 
 /*!
@@ -540,9 +544,9 @@ inline int aor_to_contact(str* aor, str* contact) {
  */
 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) && VALID_CONTACT(ptr, act_time)) {//check validity
+        if ((_c->len == ptr->c.len) && !memcmp(_c->s, ptr->c.s, _c->len)) {//check validity
             return ptr;
         }
         ptr = ptr->next;
@@ -559,11 +563,11 @@ static inline struct ucontact* contact_match(unsigned int slot, str* _c) {
 static inline struct ucontact* contact_port_ip_match(unsigned int slot, str* _c) {
     ucontact_t* ptr = contact_list->slot[slot].first;
     str string_ip_port, contact_ip_port;
-    aor_to_contact(_c, &string_ip_port);//strip userpart from test contact
+    aor_to_contact(_c, &string_ip_port); //strip userpart from test contact
 
     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) && VALID_CONTACT(ptr, act_time)) {
+        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)) {
             return ptr;
         }
 
@@ -586,8 +590,7 @@ static inline struct ucontact* contact_callid_match(unsigned int slot,
     while (ptr) {
         if ((_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)
-                && VALID_CONTACT(ptr, act_time)) {
+                && !memcmp(_callid->s, ptr->callid.s, _callid->len)) {
             return ptr;
         }
         ptr = ptr->next;
@@ -613,7 +616,7 @@ static inline struct ucontact* contact_path_match(unsigned int slot, str* _c, st
                 && !memcmp(_c->s, ptr->c.s, _c->len)
                 && !memcmp(_path->s, ptr->path.s, _path->len)
                 && VALID_CONTACT(ptr, act_time)
-		) {
+                ) {
             return ptr;
         }
 
@@ -642,10 +645,10 @@ int get_ucontact(impurecord_t* _r, str* _c, str* _callid, str* _path, int _cseq,
 
     sl = core_hash(_c, 0, contact_list->size);
     LM_DBG("looking for contact [%.*s] in slot %d\n", _c->len, _c->s, sl);
-    lock_contact_slot_i(sl);
-    
     get_act_time();
-    
+
+    lock_contact_slot_i(sl);
+
     switch (matching_mode) {
         case CONTACT_ONLY:
             ptr = contact_match(sl, _c);
@@ -656,41 +659,33 @@ int get_ucontact(impurecord_t* _r, str* _c, str* _callid, str* _path, int _cseq,
             break;
         case CONTACT_PATH:
             ptr = contact_path_match(sl, _c, _path);
-	    break;
-	case CONTACT_PORT_IP_ONLY:
-	    ptr = contact_port_ip_match(sl, _c);
-	    break;
+            break;
+        case CONTACT_PORT_IP_ONLY:
+            ptr = contact_port_ip_match(sl, _c);
+            break;
         default:
             LM_CRIT("unknown matching_mode %d\n", matching_mode);
             unlock_contact_slot_i(sl);
-	    return -1;
+            return -1;
     }
-    
-    unlock_contact_slot_i(sl);	/*TODO: we probably need to ref count here..... */
 
     if (ptr) {
-	LM_DBG("have partially found a contact\n");
+        LM_DBG("have partially found a contact\n");
         /* found -> check callid and cseq */
         if (!with_callid || (_callid && ptr->callid.len == _callid->len
                 && memcmp(_callid->s, ptr->callid.s, _callid->len) == 0)) {
             if (_cseq < ptr->cseq) {
-		LM_DBG("cseq less than expected\n");
-//                return -1;
-	    }
-//            if (_cseq == ptr->cseq) {
-//                get_act_time();
-//                return (ptr->last_modified + cseq_delay > act_time) ? -2 : -1;
-//            }
-	    
-	    
+                LM_DBG("cseq less than expected\n");
+            }
+            
         }
-	LM_DBG("contact found p=[%p], aor:[%.*s] and contact:[%.*s]\n", ptr, ptr->aor.len, ptr->aor.s, ptr->c.len, ptr->c.s);
-	
-	ptr->ref_count++;
+        LM_DBG("contact found p=[%p], aor:[%.*s] and contact:[%.*s]\n", ptr, ptr->aor.len, ptr->aor.s, ptr->c.len, ptr->c.s);
+        ref_contact_unsafe(ptr);
         *_co = ptr;
-	
+        unlock_contact_slot_i(sl); /*TODO: we probably need to ref count here..... */
         return 0;
     }
+    unlock_contact_slot_i(sl);
 
     return 1;
 }
@@ -777,85 +772,120 @@ void free_ims_subscription_data(ims_subscription *s) {
 
 }
 
-/* update an existing impurecord. if one doesnt exist it will be created.
- * make sure yuo lock the domain before calling this and unlock it afterwards
- * return: 0 on success, -1 on failure
+/* Still needs to be implemented */
+int compare_subscription(ims_subscription* new, ims_subscription* orig) {
+    int i, j;
+    LM_DBG("Comparing subscription for IMPI [%.*s]\n", orig->private_identity.len, orig->private_identity.s);
+    for (i = 0; i < orig->service_profiles_cnt; i++) {
+
+        for (j = 0; j < orig->service_profiles[i].public_identities_cnt; j++) {
+
+        }
+    }
+
+    return 0;
+}
+
+/**
+ * @brief update an existing impurecord. if one doesn't exist it will be created. assumes the domain is locked
+ * @param _d
+ * @param public_identity only used if impu_rec is null
+ * @param impu_rec if passed in we use this as the record and we assume caller has already done locking on the domain...
+ * @param reg_state
+ * @param send_sar_on_delete
+ * @param barring
+ * @param is_primary
+ * @param s
+ * @param ccf1
+ * @param ccf2
+ * @param ecf1
+ * @param ecf2
+ * @param _r
+ * @return 0 on success (domain will remain locked)
  */
-int update_impurecord(struct udomain* _d, str* public_identity, int reg_state, int send_sar_on_delete, int barring, int is_primary, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r) {
+int update_impurecord(struct udomain* _d, str* public_identity, impurecord_t* impu_rec, int reg_state, int send_sar_on_delete, int barring, int is_primary, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r) {
     int res;
-    int subscription_hash, sl;
-    struct ims_subscription_s* subscription, *subs_ptr;
+    struct ims_subscription_s* subscription, *subs_ptr = 0;
+    int leave_slot_locked = 1;
+    int subscription_locked = 0;
+    str private_identity = {0, 0};
+    str* impu_str = public_identity;
+
+    //make usre we have IMPU or enough data to find it...
+    if (!impu_rec && (!public_identity || !public_identity->len || !public_identity->s)) {
+        LM_WARN("can't call update_impurecord with no details of IMPU..n");
+        return -1;
+    }
 
     /* before we get started let's check if we already have subscription data for this impi */
     if (s && *s) {
         subs_ptr = (*s);
-        subscription_hash = core_hash(&(*s)->private_identity, 0, 0);
-        sl = subscription_hash & (subs_hash_size - 1);
-        lock_get(ims_subscription_list->slot[sl].lock);
-        subscription = ims_subscription_list->slot[sl].first;
-        if (!subscription) {
+        res = get_subscription(&(*s)->private_identity, &subscription, leave_slot_locked); //leave slot locked in case we need to add.... don't want racing adds
+        if (res != 0) {
             LM_DBG("No subscription yet for [%.*s]... adding\n", (*s)->private_identity.len, (*s)->private_identity.s);
-            //(*s)->ref_count++;  //bump subscription - we're currently referencing it
-            subs_slot_add(&ims_subscription_list->slot[sl], subs_ptr);
+            ref_subscription_unsafe(subs_ptr); //we reference coz we are using it - will be unreferenced later.
+            add_subscription_unsafe(subs_ptr);
+            unlock_subscription_slot(subs_ptr->sl);
         } else {
-            /* check through entries if any match */
-            while (subscription) {
-                if ((subs_ptr->private_identity.len == subscription->private_identity.len) && (memcmp(subs_ptr->private_identity.s, subscription->private_identity.s, subscription->private_identity.len) == 0)) {
-                    LM_DBG("found an existing subscription for IMPI [%.*s]\n", subs_ptr->private_identity.len, subs_ptr->private_identity.s);
-                    //reuse the existing one... TODO - maybe update it with the latest profile... (if it has changed)
-                    subs_ptr = subscription;
-                    //subscription->ref_count++;    //we don't bump ref now becuase this impu could already be referencing this subscription
-                    break;
-                }
-                subscription = subscription->next;
-            }
-            if (subscription == NULL) {
-                LM_DBG("could not find existing subscription for IMPI [%.*s]... adding\n", (*s)->private_identity.len, (*s)->private_identity.s);
-                //subs_ptr->ref_count++;  //bump ref coz we're currently reffing it
-                subs_slot_add(&ims_subscription_list->slot[sl], subs_ptr);
-            }
+            //TODO: we may want to do a deep comparison of the subscription and update....
+            subs_ptr = subscription;
         }
-        lock_release(ims_subscription_list->slot[sl].lock);
+        lock_subscription(subs_ptr);
+        subscription_locked = 1;
+        private_identity = (*s)->private_identity;
     }
 
-
-    //    r = _d->table[sl].first;
-    res = get_impurecord(_d, public_identity, _r);
-    if (res != 0) {
-        if (reg_state != IMPU_NOT_REGISTERED && s) {
-            LM_DBG("No existing impu record for <%.*s>.... creating new one\n", public_identity->len, public_identity->s);
-            res = insert_impurecord(_d, public_identity, reg_state, barring, &subs_ptr, ccf1, ccf2, ecf1, ecf2, _r);
-            if (res != 0) {
-                LM_ERR("Unable to insert new IMPU for <%.*s>\n", public_identity->len, public_identity->s);
-                return -1;
+    if (impu_rec) {
+        LM_DBG("We already have impurecord....\n");
+        (*_r) = impu_rec;
+        impu_str = &(*_r)->public_identity;
+    } else {
+        res = get_impurecord(_d, impu_str, _r); //return with lock on the domain
+        if (res != 0) {
+            if (reg_state != IMPU_NOT_REGISTERED && s) {
+                LM_DBG("No existing impu record for <%.*s>.... creating new one\n", impu_str->len, impu_str->s);
+                res = insert_impurecord(_d, impu_str, &private_identity, reg_state, barring, &subs_ptr, ccf1, ccf2, ecf1, ecf2, _r);
+                if (res != 0) {
+                    LM_ERR("Unable to insert new IMPU for <%.*s>\n", impu_str->len, impu_str->s);
+                    //                    unlock_udomain(_d, impu_str);
+                    goto error;
+                } else {
+                    //for the first time we create an IMPU we must set the primary record (we don't worry about it on updates - ignored)
+                    (*_r)->is_primary = is_primary; //TODO = this should prob move to insert_impurecord fn
+                    if (reg_state == IMPU_UNREGISTERED) {
+                        //update unreg expiry so the unreg record is not stored 'forever'
+                        (*_r)->expires = time(NULL) + unreg_validity;
+                    }
+                    run_ul_callbacks(NULL, UL_IMPU_INSERT, *_r, NULL);
+                    if (subscription_locked) {
+                        unref_subscription_unsafe(subs_ptr);
+                        unlock_subscription(subs_ptr);
+                    }
+                    //                    unlock_udomain(_d, impu_str);
+                    return 0;
+                }
             } else {
-                //for the first time we create an IMPU we must set the primary record (we don't worry about it on updates - ignored)
-                (*_r)->is_primary = is_primary; //TODO = this should prob move to insert_impurecord fn
-                if (reg_state == IMPU_UNREGISTERED) {
-                    //update unreg expiry so the unreg record is not stored 'forever'
-                    (*_r)->expires = time(NULL) + unreg_validity;
+                LM_DBG("no IMPU found to update and data not valid to create new one - not a problem record was probably removed as it has no contacts\n");
+                if (subscription_locked) {
+                    unref_subscription_unsafe(subs_ptr);
+                    unlock_subscription(subs_ptr);
                 }
-                run_ul_callbacks(NULL, UL_IMPU_INSERT, *_r, NULL);
                 return 0;
             }
-        } else {
-            LM_DBG("no IMPU found to update and data not valid to create new one - not a problem record was probably removed as it has no contacts\n");
-            return 0;
         }
-
     }
 
     //if we get here, we have a record to update
-    LM_DBG("updating IMPU record with public identity for <%.*s>\n", public_identity->len, public_identity->s);
+    LM_DBG("updating IMPU record with public identity for <%.*s>\n", impu_str->len, impu_str->s);
     (*_r)->reg_state = reg_state;
     if (reg_state == IMPU_UNREGISTERED) {
         //update unreg expiry so the unreg record is not stored 'forever'
         (*_r)->expires = time(NULL) + unreg_validity;
     }
     if (barring >= 0) (*_r)->barring = barring;
-    
+
     if (send_sar_on_delete >= 0) (*_r)->send_sar_on_delete = send_sar_on_delete;
-    
+
     if (ccf1) {
         if ((*_r)->ccf1.s)
             shm_free((*_r)->ccf1.s);
@@ -877,155 +907,174 @@ int update_impurecord(struct udomain* _d, str* public_identity, int reg_state, i
         STR_SHM_DUP((*_r)->ecf2, *ecf2, "SHM ECF2");
     }
 
-    if (s) {
+    if (subs_ptr) {
         LM_DBG("IMS subscription passed into update_impurecord\n");
         if ((*_r)->s != subs_ptr) {
             LM_DBG("new subscription for IMPU... swapping - TODO need to unref the old one...and then ref the new one\n");
-            unref_subscription_unsafe((*_r)->s);
+            unref_subscription((*_r)->s); //different subscription which we don't have lock on yet.
             ref_subscription_unsafe(subs_ptr);
             (*_r)->s = subs_ptr;
         } else {
-            if (s && *s) {
-                LM_DBG("new subscription is the same as the old one....not doing anything");
+            LM_DBG("new subscription is the same as the old one....not doing anything");
+            //check that the service profile and associated impus are in the subscription, if not, add...
+            if (compare_subscription(subs_ptr, *s) != 0) {
+                LM_WARN("TODO: There is a new service profile we need to add to the subscription\n");
             }
         }
     }
 
     run_ul_callbacks((*_r)->cbs, UL_IMPU_UPDATE, *_r, NULL);
-    
+
     if (db_mode == WRITE_THROUGH && db_insert_impurecord(_d, &(*_r)->public_identity, (*_r)->reg_state, (*_r)->barring, &(*_r)->s, &(*_r)->ccf1, &(*_r)->ccf2, &(*_r)->ecf1, &(*_r)->ecf2, _r) != 0) {
-	LM_ERR("error inserting IMPU [%.*s] into db... continuing", (*_r)->public_identity.len, (*_r)->public_identity.s);
+        LM_ERR("error inserting IMPU [%.*s] into db... continuing", (*_r)->public_identity.len, (*_r)->public_identity.s);
     }
-    
+
+    if (subscription_locked) {
+        unref_subscription_unsafe(subs_ptr);
+        unlock_subscription(subs_ptr);
+    }
+
     return 0;
 
 out_of_memory:
-    unlock_udomain(_d, public_identity);
+
+    error :
+    if (subscription_locked) {
+        unref_subscription_unsafe(subs_ptr);
+        unlock_subscription(subs_ptr);
+    }
+
     return -1;
 }
 
 /* link contact to impu 
     must be called with lock on domain (IMPU) as well as lock on contact_slot 
  */
-int link_contact_to_impu(impurecord_t* impu, ucontact_t* contact, int write_to_db)
-{
+int link_contact_to_impu(impurecord_t* impu, ucontact_t* contact, int write_to_db) {
     ucontact_t* ptr;
     int i;
-    i=0;
-    int overwrite=0;
-    ptr=impu->newcontacts[i];
+    i = 0;
+    int overwrite = 0;
+    ptr = impu->newcontacts[i];
 
-    
-    
-    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);
-	    return 0;
-	}
-	i++;
-	ptr = impu->newcontacts[i];
+    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);
+            return 0;
+        }
+        i++;
+        ptr = impu->newcontacts[i];
     }
-    
-    if ((maxcontact_behaviour>0) && (maxcontact>0) && (maxcontact < (i+1))) {
-	LM_DBG("Need to overwrite oldest contact at position %d\n", i);
-	i = maxcontact-1;
-	overwrite = 1;
+
+    if ((maxcontact_behaviour > 0) && (maxcontact > 0) && (maxcontact < (i + 1))) {
+        LM_DBG("Need to overwrite oldest contact at position %d\n", i);
+        i = maxcontact - 1;
+        overwrite = 1;
     }
-    
-    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);    //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;
-	
-	contact->ref_count++;
-	LM_DBG("number of contacts for IMPU [%.*s] is %d\n", impu->public_identity.len, impu->public_identity.s, impu->num_contacts);
-	if (write_to_db && db_mode == WRITE_THROUGH && db_link_contact_to_impu(impu, contact) !=0) {
-	    LM_ERR("Failed to update DB linking contact [%.*s] to IMPU [%.*s]...continuing but db will be out of sync!\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s);
-	};
+
+    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); //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);
+        LM_DBG("number of contacts for IMPU [%.*s] is %d\n", impu->public_identity.len, impu->public_identity.s, impu->num_contacts);
+        if (write_to_db && db_mode == WRITE_THROUGH && db_link_contact_to_impu(impu, contact) != 0) {
+            LM_ERR("Failed to update DB linking contact [%.*s] to IMPU [%.*s]...continuing but db will be out of sync!\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s);
+        };
     } else {
-	LM_DBG("unable to link contact to impu as too many links already > %d\n", MAX_CONTACTS_PER_IMPU);
-	return -1;
+        LM_DBG("unable to link contact to impu as too many links already > %d\n", MAX_CONTACTS_PER_IMPU);
+        return -1;
     }
-    
+
     return 0;
 }
 
-/* link contact to impu 
-    must be called with lock on domain (IMPU) as well as lock on contact_slot 
- */
-int unlink_contact_from_impu(impurecord_t* impu, ucontact_t* contact, int write_to_db)
-{
+int unlink_contact_from_impu(impurecord_t* impu, ucontact_t* contact, int write_to_db) {
     ucontact_t* ptr;
     int i;
-    i=0;
-    int found=0;
-    ptr=impu->newcontacts[i];
+    i = 0;
+    int found = 0;
+    ptr = impu->newcontacts[i];
 
-    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);    
-    
-    while (i<MAX_CONTACTS_PER_IMPU && ptr) {
-	if (found) {
-	    //shift all later pointers forward by 1
-	    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);
-		
-		if (exists_ulcb_type(impu->cbs, UL_IMPU_DELETE_CONTACT)) {
-		    LM_DBG("Running callback UL_IMPU_DELETE_CONTACT for contact [%.*s] and impu [%.*s]\n", impu->public_identity.len, impu->public_identity.s, ptr->c.len, ptr->c.s);
-		    run_ul_callbacks(impu->cbs, UL_IMPU_DELETE_CONTACT, impu, ptr);
-		}
-
-		found = 1;
-		impu->newcontacts[i]=0;
-		impu->num_contacts--;
-		LM_DBG("decrementing ref count on contact [%.*s] to %d\n", contact->c.len, contact->c.s, contact->ref_count);
-		contact->ref_count--;	//TODO - should we lock the actual ucontact struct?
-		if (write_to_db && db_mode == WRITE_THROUGH && db_unlink_contact_from_impu(impu, contact) != 0) {
-		    LM_ERR("Failed to un-link DB contact [%.*s] from IMPU [%.*s]...continuing but db will be out of sync!\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s);
-		}
-	    }
-	}
-	i++;
-	ptr = impu->newcontacts[i];
+    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);
+
+    while (i < MAX_CONTACTS_PER_IMPU && ptr) {
+        if (found) {
+            //shift all later pointers forward by 1
+            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);
+
+                if (exists_ulcb_type(impu->cbs, UL_IMPU_DELETE_CONTACT)) {
+                    LM_DBG("Running callback UL_IMPU_DELETE_CONTACT for contact [%.*s] and impu [%.*s]\n", ptr->c.len, ptr->c.s, impu->public_identity.len, impu->public_identity.s);
+                    run_ul_callbacks(impu->cbs, UL_IMPU_DELETE_CONTACT, impu, ptr);
+                }
+
+                found = 1;
+                impu->newcontacts[i] = 0;
+                impu->num_contacts--;
+                unref_contact_unsafe(contact); //should we lock the actual contact? safe version maybe?
+
+                if (write_to_db && db_mode == WRITE_THROUGH && db_unlink_contact_from_impu(impu, contact) != 0) {
+                    LM_ERR("Failed to un-link DB contact [%.*s] from IMPU [%.*s]...continuing but db will be out of sync!\n", contact->c.len, contact->c.s, impu->public_identity.len, impu->public_identity.s);
+                }
+            }
+        }
+        i++;
+        ptr = impu->newcontacts[i];
     }
 
     if (found && i < MAX_CONTACTS_PER_IMPU) {
-	LM_DBG("zero'ing last pointer to contact in the list\n");
-	impu->newcontacts[i-1]=0;
+        LM_DBG("zero'ing last pointer to contact in the list\n");
+        impu->newcontacts[i - 1] = 0;
     } 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);
+        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);
     }
-    
+
     return 0;
 }
 
 void ref_subscription_unsafe(ims_subscription* s) {
     LM_DBG("Reffing subscription [%.*s] - was [%d]\n", s->private_identity.len, s->private_identity.s, s->ref_count);
-    lock_subscription(s);
     s->ref_count++;
-    unlock_subscription(s);
 }
 
+/**
+ * @brief unref a subscription - assume slot and subsription locked!
+ * @param s
+ */
 void unref_subscription_unsafe(ims_subscription* s) {
     int sl;
-    unsigned int subscription_hash;
-    LM_DBG("un-Reffing subscription [%.*s] - was [%d]\n", s->private_identity.len, s->private_identity.s, s->ref_count);
 
-    subscription_hash = core_hash(&s->private_identity, 0, 0);
-    sl = subscription_hash & (subs_hash_size - 1);
-    lock_get(ims_subscription_list->slot[sl].lock);
-    lock_subscription(s);
+    LM_DBG("un-reffing subscription [%.*s] - was [%d]\n", s->private_identity.len, s->private_identity.s, s->ref_count);
     s->ref_count--;
-    if (s->ref_count == 0) {
+    if (s->ref_count == 0 && (s->sl >= 0)) { //-1 as sl means the subscription was never added to the list
+        sl = s->sl;
         subs_slot_rem(&ims_subscription_list->slot[sl], s);
-        free_ims_subscription_data(s);
-    } else {
-        unlock_subscription(s);
+        delete_subscription(s);
+        s = 0;
     }
-    lock_release(ims_subscription_list->slot[sl].lock);
+}
+
+void ref_subscription(ims_subscription* s) {
+    lock_subscription(s);
+    ref_subscription_unsafe(s);
+    unlock_subscription(s);
+}
+
+/**
+ * @brief unref subscription safely - assume no lock on subscription or subscription slot
+ * @param s
+ */
+void unref_subscription(ims_subscription* s) {
+    int ref;
+    lock_subscription(s);
+    ref = s->ref_count;
+    unref_subscription_unsafe(s);
+    if (ref > 1)
+        unlock_subscription(s);
 }

modules/ims_usrloc_scscf/impurecord.h

@@ -65,7 +65,7 @@ struct hslot; /*!< Hash table slot */
  * \param _r pointer to the new record
  * \return 0 on success, negative on failure
  */
-int new_impurecord(str* _dom, str* public_identity, int reg_state, int barring, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, impurecord_t** _r);
+int new_impurecord(str* _dom, str* public_identity, str* private_identity, int reg_state, int barring, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, impurecord_t** _r);
 
 
 /*!
@@ -170,13 +170,15 @@ int get_ucontact(impurecord_t* _r, str* _c, str* _callid, str* _path,
 		int _cseq,
 		struct ucontact** _co);
 
-int update_impurecord(struct udomain* _d, str* public_identity, int reg_state, int send_sar_on_delete, int barring, int is_primary, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r);
+int update_impurecord(struct udomain* _d, str* public_identity, impurecord_t* impu_rec, int reg_state, int send_sar_on_delete, int barring, int is_primary, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r);
 
 int link_contact_to_impu(impurecord_t* impu, ucontact_t* contact, int write_to_db);
 int unlink_contact_from_impu(impurecord_t* impu, ucontact_t* contact, int write_to_db);
 
 void ref_subscription_unsafe(ims_subscription* s);
 void unref_subscription_unsafe(ims_subscription* s);
+void ref_subscription(ims_subscription* s);
+void unref_subscription(ims_subscription* s);
 
 #endif
 

modules/ims_usrloc_scscf/subscribe.c

@@ -209,14 +209,10 @@ str get_presentity_from_subscriber_dialog(str *callid, str *to_tag, str *from_ta
     str pres_uri = {0,0};
     
     hash_code = core_hash(callid, to_tag, sub_dialog_hash_size);
-    
     /* search the record in hash table */
     lock_get(&sub_dialog_table[hash_code].lock);
-
     LM_DBG("Searching sub dialog hash info with call_id: <%.*s> and ttag <%.*s> and ftag <%.*s> and hash code <%d>", callid->len, callid->s, to_tag->len, to_tag->s, from_tag->len, from_tag->s, hash_code);
-    
-    s= pres_search_shtable(sub_dialog_table, *callid,
-		    *to_tag, *from_tag, hash_code);
+    s = pres_search_shtable(sub_dialog_table, *callid, *to_tag, *from_tag, hash_code);
     if(s== NULL)
     {
 	    LM_DBG("Subscriber dialog record not found in hash table\n");

modules/ims_usrloc_scscf/ucontact.c

@@ -147,7 +147,7 @@ ucontact_t* new_ucontact(str* _dom, str* _aor, str* _contact, ucontact_info_t* _
     }
     
     LM_DBG("generating hash based on [%.*s]\n", _contact->len, _contact->s);
-    c->contact_hash = core_hash(_contact, 0, contact_list->size);
+    c->sl = core_hash(_contact, 0, contact_list->size);
     c->ref_count = 1;
     c->expires = _ci->expires;
     c->q = _ci->q;
@@ -531,7 +531,7 @@ int remove_dialog_data_from_contact(ucontact_t* _c, unsigned int h_entry, unsign
 }
 
 void release_ucontact(struct ucontact* _c) {
-    lock_contact_slot_i(_c->contact_hash);
-    _c->ref_count--;
-    unlock_contact_slot_i(_c->contact_hash);
+    lock_contact_slot_i(_c->sl);
+    unref_contact_unsafe(_c);
+    unlock_contact_slot_i(_c->sl);
 }

modules/ims_usrloc_scscf/udomain.c

@@ -68,11 +68,14 @@
 #include "usrloc_db.h"
 #include "contact_hslot.h"
 #include "ul_scscf_stats.h"
+#include "hslot_sp.h"
+#include "dlist.h"
 
 extern int unreg_validity;
 extern int db_mode;
 struct contact_list* contact_list;
-extern struct ul_scscf_counters_h ul_scscf_cnts_h;
+struct ims_subscription_list* ims_subscription_list;
+extern int subs_hash_size;
 
 /*!
  * \brief Create a new domain structure
@@ -91,21 +94,21 @@ int new_udomain(str* _n, int _s, udomain_t** _d) {
      */
     *_d = (udomain_t*) shm_malloc(sizeof (udomain_t));
     if (!(*_d)) {
-	LM_ERR("new_udomain(): No memory left\n");
-	goto error0;
+        LM_ERR("new_udomain(): No memory left\n");
+        goto error0;
     }
     memset(*_d, 0, sizeof (udomain_t));
 
     (*_d)->table = (hslot_t*) shm_malloc(sizeof (hslot_t) * _s);
     if (!(*_d)->table) {
-	LM_ERR("no memory left 2\n");
-	goto error1;
+        LM_ERR("no memory left 2\n");
+        goto error1;
     }
 
     (*_d)->name = _n;
 
     for (i = 0; i < _s; i++) {
-	init_slot(*_d, &((*_d)->table[i]), i);
+        init_slot(*_d, &((*_d)->table[i]), i);
     }
 
     (*_d)->size = _s;
@@ -126,12 +129,12 @@ void free_udomain(udomain_t* _d) {
     int i;
 
     if (_d->table) {
-	for (i = 0; i < _d->size; i++) {
-	    lock_ulslot(_d, i);
-	    deinit_slot(_d->table + i);
-	    unlock_ulslot(_d, i);
-	}
-	shm_free(_d->table);
+        for (i = 0; i < _d->size; i++) {
+            lock_ulslot(_d, i);
+            deinit_slot(_d->table + i);
+            unlock_ulslot(_d, i);
+        }
+        shm_free(_d->table);
     }
     shm_free(_d);
 }
@@ -162,19 +165,18 @@ void print_udomain(FILE* _f, udomain_t* _d) {
     fprintf(_f, "name : '%.*s'\n", _d->name->len, ZSW(_d->name->s));
     fprintf(_f, "size : %d\n", _d->size);
     fprintf(_f, "table: %p\n", _d->table);
-    /*fprintf(_f, "lock : %d\n", _d->lock); -- can be a structure --andrei*/
     fprintf(_f, "\n");
     for (i = 0; i < _d->size; i++) {
-	r = _d->table[i].first;
-	n += _d->table[i].n;
-	if (max < _d->table[i].n) {
-	    max = _d->table[i].n;
-	    slot = i;
-	}
-	while (r) {
-	    print_impurecord(_f, r);
-	    r = r->next;
-	}
+        r = _d->table[i].first;
+        n += _d->table[i].n;
+        if (max < _d->table[i].n) {
+            max = _d->table[i].n;
+            slot = i;
+        }
+        while (r) {
+            print_impurecord(_f, r);
+            r = r->next;
+        }
     }
     fprintf(_f, "\nMax slot: %d (%d/%d)\n", max, slot, n);
     fprintf(_f, "\n---/Domain---\n");
@@ -185,8 +187,8 @@ inline int time2str(time_t _v, char* _s, int* _l) {
     int l;
 
     if ((!_s) || (!_l) || (*_l < 2)) {
-	LM_ERR("Invalid parameter value\n");
-	return -1;
+        LM_ERR("Invalid parameter value\n");
+        return -1;
     }
 
     *_s++ = '\'';
@@ -196,11 +198,11 @@ inline int time2str(time_t _v, char* _s, int* _l) {
     l = strftime(_s, *_l - 1, "%Y-%m-%d %H:%M:%S", t);
 
     if (l == 0) {
-	LM_ERR("Error during time conversion\n");
-	/* the value of _s is now unspecified */
-	_s = NULL;
-	_l = 0;
-	return -1;
+        LM_ERR("Error during time conversion\n");
+        /* the value of _s is now unspecified */
+        _s = NULL;
+        _l = 0;
+        return -1;
     }
     *_l = l;
 
@@ -216,17 +218,16 @@ inline int time2str(time_t _v, char* _s, int* _l) {
  * \param _r new created record
  * \return 0 on success, -1 on failure
  */
-int mem_insert_impurecord(struct udomain* _d, str* public_identity, int reg_state, int barring,
-	ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2,
-	struct impurecord** _r) {
+int mem_insert_impurecord(struct udomain* _d, str* public_identity, str* private_identity, int reg_state, int barring,
+        ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2,
+        struct impurecord** _r) {
     int sl;
 
-    if (new_impurecord(_d->name, public_identity, reg_state, barring, s, ccf1, ccf2, ecf1,
-	    ecf2, _r) < 0) {
-	LM_ERR("creating impurecord failed\n");
-	return -1;
+    if (new_impurecord(_d->name, public_identity, private_identity, reg_state, barring, s, ccf1, ccf2, ecf1,
+            ecf2, _r) < 0) {
+        LM_ERR("creating impurecord failed\n");
+        return -1;
     }
-    LM_DBG("Successfully parsed user data\n");
 
     sl = ((*_r)->aorhash) & (_d->size - 1);
     slot_add(&_d->table[sl], *_r);
@@ -254,126 +255,50 @@ void mem_delete_impurecord(udomain_t* _d, struct impurecord* _r) {
  */
 void mem_timer_udomain(udomain_t* _d) {
     struct impurecord* ptr, *t;
-    struct ucontact* contact_ptr, *tmp_contact_ptr;
+    struct ucontact* contact_ptr;
     int i;
 
     //go through contacts first
     LM_DBG("*** mem_timer_udomain - checking contacts - START ***\n");
-    
     for (i = 0; i < contact_list->size; i++) {
-#ifdef EXTRA_DEBUG
-	LM_DBG("looking for contacts in slot %d\n", i);
-#endif
-	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\n", i, contact_ptr->aor.len, contact_ptr->aor.s, contact_ptr->c.len, contact_ptr->c.s, (double) contact_ptr->expires - time(NULL), contact_ptr->ref_count);
-	    if (contact_ptr->ref_count <= 0) {
-		LM_DBG("Deleting contact [%.*s]\n", contact_ptr->c.len, contact_ptr->c.s);
-		tmp_contact_ptr = contact_ptr->next;
-		delete_ucontact(contact_ptr);
-		contact_ptr = tmp_contact_ptr;
-	    } else {
-		contact_ptr = contact_ptr->next;
-	    }
-	}
-	unlock_contact_slot_i(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\n", i, contact_ptr->aor.len, contact_ptr->aor.s, contact_ptr->c.len, contact_ptr->c.s, (double) contact_ptr->expires - time(NULL), contact_ptr->ref_count);
+		//contacts are now deleted during impurecord processing
+            contact_ptr = contact_ptr->next;
+        } 
+        unlock_contact_slot_i(i);
     }
-    
     LM_DBG("*** mem_timer_udomain - checking contacts - FINISHED ***\n");
 
     int temp = 0;
 
     LM_DBG("*** mem_timer_udomain - checking IMPUs - START ***\n");
     for (i = 0; i < _d->size; i++) {
-	lock_ulslot(_d, i);
-	ptr = _d->table[i].first;
-	temp = 0;
-	while (ptr) {
-	    temp = 1;
+        lock_ulslot(_d, i);
+        ptr = _d->table[i].first;
+        temp = 0;
+        while (ptr) {
+            temp = 1;
 #ifdef EXTRA_DEBUG
-	    LM_DBG("ULSLOT %d LOCKED\n", i);
+            LM_DBG("ULSLOT %d LOCKED\n", i);
 #endif
-	    t = ptr;
-	    ptr = ptr->next;
-	    timer_impurecord(t);
-
-//	    			if (t->reg_state == IMPU_NOT_REGISTERED && t->shead == 0) {
-//	    				//remove it - housekeeping - not sure why its still here...?
-//	    				if (exists_ulcb_type(t->cbs, UL_IMPU_NR_DELETE))
-//	    					run_ul_callbacks(t->cbs, UL_IMPU_NR_DELETE, t, NULL);
-//	    					    
-//	    				LM_DBG("about to delete impurecord\n");
-//	    				delete_impurecord(_d, &t->public_identity, t);
-//	    			} //else if (t->reg_state == IMPU_UNREGISTERED) {//Remove IMPU record if it is in state IMPU_UNREGISTERED and has expired
-	    //			    
-	    //				if (time_now >= t->expires) {//check here and only remove if no subscribes - if there is a subscribe then bump the validity by unreg_validity
-	    //				    if(t->shead != 0){
-	    //					LM_DBG("This impurecord still has subscriptions - extending the expiry");
-	    //					t->expires = time(NULL) + unreg_validity;
-	    //				    } else {
-	    //					if (exists_ulcb_type(t->cbs, UL_IMPU_UNREG_EXPIRED))
-	    //						run_ul_callbacks(t->cbs, UL_IMPU_UNREG_EXPIRED, t, NULL);
-	    //					LM_DBG("about to delete impurecord\n");
-	    //					delete_impurecord(_d, &t->public_identity, t);
-	    //				    }
-	    //				}
-	    //			//} else if (t->reg_state != IMPU_UNREGISTERED && t->contacts == 0) { /* Remove the entire record if it is empty IFF it is not an UNREGISTERED RECORD */
-	    //			} else if (t->reg_state != IMPU_UNREGISTERED && t->num_contacts == 0 && t->shead == 0) { /* Remove the entire record if it is empty IFF it is not an UNREGISTERED RECORD */
-	    //																								/* TS 23.228 5.3.2.1 (release 11) */
-	    //				//need a way of distinguishing between deletes that need a SAR (expired) and deletes that do not need a SAR (explicit de reg)
-	    //				//we only want to send one SAR for each implicit IMPU set
-	    //				//make sure all IMPU's associated with this set are de-registered before calling the callbacks
-	    //				int first=1;
-	    //				int this_is_first = 0;
-	    //
-	    //				lock_get(t->s->lock);
-	    //				for (k = 0; k < t->s->service_profiles_cnt; k++){
-	    //					for (j = 0;j < t->s->service_profiles[k].public_identities_cnt;j++) {
-	    //						impu = &(t->s->service_profiles[k].public_identities[j]);
-	    //
-	    //						sl = core_hash(&impu->public_identity, 0, _d->size);
-	    //						if (sl != i)
-	    //							lock_udomain(_d, &impu->public_identity);
-	    //
-	    //						if (first) {
-	    //							first = 0; //dont do anything - we will leave this impu to be processed as normal
-	    //							if (!strncmp(impu->public_identity.s, t->public_identity.s, t->public_identity.len)) {
-	    //								//we are the first in the implicit set
-	    //								this_is_first = 1;
-	    //							}
-	    //						} else {
-	    //							//set all other implicits to not registered
-	    //							if (update_impurecord(_d, &impu->public_identity, IMPU_NOT_REGISTERED,
-	    //														-1/*barring*/, -1 /*do not change send sar on delete */, 0/*is_primary*/, NULL, NULL, NULL, NULL, NULL, &temp_impu) != 0) {
-	    //								LM_ERR("Unable to update impurecord for <%.*s>\n", impu->public_identity.len, impu->public_identity.s);
-	    //							}
-	    //						}
-	    //						if (sl != i)
-	    //							unlock_udomain(_d, &impu->public_identity);
-	    //					}
-	    //				}
-	    //				lock_release(t->s->lock);
-	    //
-	    //				if (this_is_first) {
-	    //					//now run a normal callback on our
-	    //					if (exists_ulcb_type(t->cbs, UL_IMPU_REG_NC_DELETE))
-	    //						run_ul_callbacks(t->cbs, UL_IMPU_REG_NC_DELETE, t, NULL);
-	    //					LM_DBG("about to delete impurecord\n");
-	    //						delete_impurecord(_d, &t->public_identity, t);
-	    //				}
-	    //			}
-	}
-	if (temp) {
+            t = ptr;
+            ptr = ptr->next;
+            timer_impurecord(t);
+        }
+        if (temp) {
 #ifdef EXTRA_DEBUG
-	    LM_DBG("ULSLOT %d UN-LOCKED\n", i);
+            LM_DBG("ULSLOT %d UN-LOCKED\n", i);
 #endif
-	}
-	unlock_ulslot(_d, i);
+        }
+        unlock_ulslot(_d, i);
     }
     LM_DBG("*** mem_timer_udomain - checking IMPUs - FINISHED ***\n");
 }
 
+
 /*!
  * \brief Get lock for a domain
  * \param _d domain
@@ -382,15 +307,7 @@ void mem_timer_udomain(udomain_t* _d) {
 void lock_udomain(udomain_t* _d, str* _aor) {
     unsigned int sl;
     sl = core_hash(_aor, 0, _d->size);
-#ifdef EXTRA_DEBUG
-    LM_DBG("LOCKING UDOMAIN SLOT [%d]\n", sl);
-#endif
-
-#ifdef GEN_LOCK_T_PREFERED
-    lock_get(_d->table[sl].lock);
-#else
-    ul_lock_idx(_d->table[sl].lockidx);
-#endif
+    lock_ulslot(_d, sl);
 }
 
 /*!
@@ -401,14 +318,7 @@ void lock_udomain(udomain_t* _d, str* _aor) {
 void unlock_udomain(udomain_t* _d, str* _aor) {
     unsigned int sl;
     sl = core_hash(_aor, 0, _d->size);
-#ifdef EXTRA_DEBUG
-    LM_DBG("UN-LOCKING UDOMAIN SLOT [%d]\n", sl);
-#endif
-#ifdef GEN_LOCK_T_PREFERED
-    lock_release(_d->table[sl].lock);
-#else
-    ul_release_idx(_d->table[sl].lockidx);
-#endif
+    unlock_ulslot(_d, sl);
 }
 
 /*!
@@ -417,11 +327,18 @@ void unlock_udomain(udomain_t* _d, str* _aor) {
  * \param i slot number
  */
 void lock_ulslot(udomain_t* _d, int i) {
-#ifdef GEN_LOCK_T_PREFERED
-    lock_get(_d->table[i].lock);
-#else
-    ul_lock_idx(_d->table[i].lockidx);
+#ifdef EXTRA_DEBUG
+    LM_DBG("LOCKING UDOMAIN SLOT [%d]\n", i);
 #endif
+    int mypid;
+    mypid = my_pid();
+    if (likely(atomic_get(&_d->table[i].locker_pid) != mypid)) {
+        lock_get(_d->table[i].lock);
+        atomic_set(&_d->table[i].locker_pid, mypid);
+    } else {
+        /* locked within the same process that executed us */
+        _d->table[i].recursive_lock_level++;
+    }
 }
 
 /*!
@@ -430,11 +347,16 @@ void lock_ulslot(udomain_t* _d, int i) {
  * \param i slot number
  */
 void unlock_ulslot(udomain_t* _d, int i) {
-#ifdef GEN_LOCK_T_PREFERED
-    lock_release(_d->table[i].lock);
-#else
-    ul_release_idx(_d->table[i].lockidx);
+#ifdef EXTRA_DEBUG
+    LM_DBG("UN-LOCKING UDOMAIN SLOT [%d]\n", i);
 #endif
+    if (likely(_d->table[i].recursive_lock_level == 0)) {
+        atomic_set(&_d->table[i].locker_pid, 0);
+        lock_release(_d->table[i].lock);
+    } else {
+        /* recursive locked => decrease lock count */
+        _d->table[i].recursive_lock_level--;
+    }
 }
 
 void lock_contact_slot(str* contact_uri) {
@@ -481,6 +403,8 @@ void lock_subscription(ims_subscription* s) {
 }
 
 void unlock_subscription(ims_subscription* s) {
+    if (s == 0)
+        return;
 #ifdef EXTRA_DEBUG
     LM_DBG("UN-LOCKING SUBSCRIPTION %p (Refcount: %d)\n", s->lock, s->ref_count);
     LM_DBG("(SUBSCRIPTION PRIVATE IDENTITY [%.*s])\n", s->private_identity.len, s->private_identity.s);
@@ -488,46 +412,75 @@ void unlock_subscription(ims_subscription* s) {
     lock_release(s->lock);
 }
 
+void lock_subscription_slot(int i) {
+#ifdef EXTRA_DEBUG
+    LM_DBG("LOCKING SUBSCRIPTION slot %d)\n", i);
+#endif
+    lock_get(ims_subscription_list->slot[i].lock);
+}
+
+void unlock_subscription_slot(int i) {
+#ifdef EXTRA_DEBUG
+    LM_DBG("UN-LOCKING SUBSCRIPTION slot %d\n", i);
+#endif
+    lock_release(ims_subscription_list->slot[i].lock);
+}
+
 /*!
- * \brief Create and insert a new record
- * \param _d domain to insert the new record
- * \param _aor address of the record
- * \param _r new created record
- * \return return 0 on success, -1 on failure
+ * \brief Create and insert a new impurecord assumes domain is locked
+ * @param _d domain to insert the new record
+ * @param public_identity IMPU of new record
+ * @param private_identity IMPI of new record
+ * @param reg_state state to insert in
+ * @param barring is impu barred or not
+ * @param s associated subscription data
+ * @param ccf1  
+ * @param ccf2
+ * @param ecf1
+ * @param ecf2
+ * @param _r pointer to returned IMPU record
+ * @return 0 on success with _r populated
  */
-int insert_impurecord(struct udomain* _d, str* public_identity, int reg_state, int barring,
-	ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2,
-	struct impurecord** _r) {
+int insert_impurecord(struct udomain* _d, str* public_identity, str* private_identity, int reg_state, int barring,
+        ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2,
+        struct impurecord** _r) {
+
+    if (s == 0 || (*s) == 0) {
+        LM_WARN("Can't insert an impurecord without it being associated to a subscription\n");
+        goto error;
+    }
+
+    if (!private_identity || !private_identity->len || !private_identity->s) {
+        LM_WARN("Can't insert an impurecord without it being associated to a subscription (private_identity\n");
+        goto error;
+    }
 
     /* check to see if we already have this subscription information in memory*/
-    if (mem_insert_impurecord(_d, public_identity, reg_state, barring, s, ccf1, ccf2, ecf1, ecf2, _r) < 0) {
-	LM_ERR("inserting record failed\n");
-	goto error;
+    if (mem_insert_impurecord(_d, public_identity, private_identity, reg_state, barring, s, ccf1, ccf2, ecf1, ecf2, _r) < 0) {
+        LM_ERR("inserting record failed\n");
+        goto error;
     }
 
     /*DB?*/
     if (db_mode == WRITE_THROUGH && db_insert_impurecord(_d, public_identity, reg_state, barring, s, ccf1, ccf2, ecf1, ecf2, _r) != 0) {
-	LM_ERR("error inserting contact into db");
-	goto error;
+        LM_ERR("error inserting contact into db");
+        goto error;
     }
 
     return 0;
 
 error:
-    //    if (s) {
-    //    	free_ims_subscription_data(s);
-    //    }
     return -1;
 }
 
 /*!
- * \brief Obtain a impurecord pointer if the impurecord exists in domain
+ * \brief Obtain a impurecord pointer if the impurecord exists in domain. You should call this function with a lock on the domain
  * \param _d domain to search the record
  * \param _aor address of record
  * \param _r new created record
- * \return 0 if a record was found, 1 if nothing could be found
+ * \return 0 if a record was found, 1 if nothing could be found (assumes caller has lock on domain)
  */
-int get_impurecord(udomain_t* _d, str* public_identity, struct impurecord** _r) {
+int get_impurecord_unsafe(udomain_t* _d, str* public_identity, struct impurecord** _r) {
     unsigned int sl, i, aorhash;
     impurecord_t* r;
 
@@ -537,63 +490,76 @@ int get_impurecord(udomain_t* _d, str* public_identity, struct impurecord** _r)
     r = _d->table[sl].first;
 
     for (i = 0; i < _d->table[sl].n; i++) {
-	if ((r->aorhash == aorhash) && (r->public_identity.len == public_identity->len)
-		&& !memcmp(r->public_identity.s, public_identity->s, public_identity->len)) {
-	    *_r = r;
-	    return 0;
-	}
+        if ((r->aorhash == aorhash) && (r->public_identity.len == public_identity->len)
+                && !memcmp(r->public_identity.s, public_identity->s, public_identity->len)) {
+            *_r = r;
+            return 0;
+        }
 
-	r = r->next;
+        r = r->next;
     }
     return 1; /* Nothing found */
 }
 
+/*!
+ * \brief Obtain a impurecord pointer if the impurecord exists in domain. domain must be locked before calling
+ * \param _d domain to search the record
+ * \param public_identity address of record
+ * \param _r returned record - null if not found
+ * \return 0 if a record was found, 1 if nothing could be found
+ */
+int get_impurecord(udomain_t* _d, str* public_identity, struct impurecord** _r) {
+    unsigned int ret;
+
+    ret = get_impurecord_unsafe(_d, public_identity, _r);
+
+    return ret;
+}
+
+/*!
+ * \brief release the lock on the impurecord - effectively the domain slot
+ * \param _d domain
+ * \param _r impurecord to release (unlock)
+ */
+void release_impurecord(udomain_t* _d, struct impurecord* _r) {
+    unlock_udomain(_d, &_r->public_identity);
+}
+
 /*!
  * \brief Delete a impurecord from domain
  * \param _d domain where the record should be deleted
- * \param _aor address of record
- * \param _r deleted record
+ * \param _aor address of record - used only if _r in next param is null
+ * \param _r deleted record to delete - if null will use the aor to search (assumed that domain is locked).
  * \return 0 on success, -1 if the record could not be deleted
  */
 int delete_impurecord(udomain_t* _d, str* _aor, struct impurecord* _r) {
-    //    struct ucontact* c;//, *t;
-
     LM_DBG("Deleting IMPURECORD [%.*s]\n", _r->public_identity.len, _r->public_identity.s);
 
     if (_r == 0) {
-	if (get_impurecord(_d, _aor, &_r) > 0) {
-	    return 0;
-	}
+        LM_DBG("no impurecord passed in - let's search\n");
+        if (get_impurecord(_d, _aor, &_r) != 0) {
+            return 0;
+        }
     }
 
-    //TODO: need to unref the contacts in the contact list (not delete them), the timer should delete all contacts that are unreffed
-    //    c = _r->contacts;
-    //    while (c) {
-    //	t = c;
-    //	c = c->next;
-    //	if (delete_ucontact(_r, t) < 0) {
-    //	    LM_ERR("deleting contact failed [%.*s]\n", c->aor.len, c->aor.s);
-    //	    return -1;
-    //	}
-    //    }
-
     if (exists_ulcb_type(_r->cbs, UL_IMPU_DELETE)) {
-	run_ul_callbacks(_r->cbs, UL_IMPU_DELETE, _r, 0);
+        run_ul_callbacks(_r->cbs, UL_IMPU_DELETE, _r, 0);
     }
 
     /*DB?*/
     if (db_mode == WRITE_THROUGH
-	    && db_delete_impurecord(_d, _r) != 0) {
-	LM_ERR("error deleting IMPU record from db");
-	return 0;
+            && db_delete_impurecord(_d, _r) != 0) {
+        LM_ERR("error deleting IMPU record from db...continuing to remove from memory\n");
     }
 
     mem_delete_impurecord(_d, _r);
+
     return 0;
 }
 
 /*
  * get all IMPUs as string from a subscription related to an impurecord. apply filter for barring (assumed to be called with lock on impurec)
+ * you should have some for of lock on the subscription (ie a reference)
  * barring-1 get all barred
  * barring-0 get all unbarred
  * barring-(-1) get all records
@@ -610,70 +576,70 @@ int get_impus_from_subscription_as_string(udomain_t* _d, impurecord_t* impu_rec,
     LM_DBG("getting IMPU subscription set\n");
 
     if (!impu_rec) {
-	LM_ERR("no impu record provided\n");
-	return 1;
+        LM_ERR("no impu record provided\n");
+        return 1;
     }
 
     if (!impu_rec->s) {
-	LM_DBG("no subscription associated with impu\n");
-	return 0;
+        LM_DBG("no subscription associated with impu\n");
+        return 0;
     }
 
     lock_subscription(impu_rec->s);
     for (i = 0; i < impu_rec->s->service_profiles_cnt; i++) {
-	for (j = 0; j < impu_rec->s->service_profiles[i].public_identities_cnt; j++) {
-	    impi = &(impu_rec->s->service_profiles[i].public_identities[j]);
-	    if (barring < 0) {
-		//get all records
-		bytes_needed += impi->public_identity.len;
-		(*num_impus)++;
-	    } else {
-		if (impi->barring == barring) {
-		    //add the record to the list
-		    bytes_needed += impi->public_identity.len;
-		    (*num_impus)++;
-		}
-	    }
-	}
+        for (j = 0; j < impu_rec->s->service_profiles[i].public_identities_cnt; j++) {
+            impi = &(impu_rec->s->service_profiles[i].public_identities[j]);
+            if (barring < 0) {
+                //get all records
+                bytes_needed += impi->public_identity.len;
+                (*num_impus)++;
+            } else {
+                if (impi->barring == barring) {
+                    //add the record to the list
+                    bytes_needed += impi->public_identity.len;
+                    (*num_impus)++;
+                }
+            }
+        }
     }
     LM_DBG("num of records returned is %d and we need %d bytes\n", *num_impus, bytes_needed);
 
     len = (sizeof (str)*(*num_impus)) + bytes_needed;
-    *impus = (str*) pkg_malloc(len);
+    *impus = (str*) pkg_malloc(len); //TODO: rather put this on the stack... dont' fragment pkg....
     if (*impus == 0) {
-	LM_ERR("no more pkg_mem\n");
-	return 0;
+        LM_ERR("no more pkg_mem\n");
+        return 0;
     }
     char* ptr = (char*) (*impus + *num_impus);
 
     //now populate the data
     count = 0;
     for (i = 0; i < impu_rec->s->service_profiles_cnt; i++) {
-	for (j = 0; j < impu_rec->s->service_profiles[i].public_identities_cnt; j++) {
-	    impi = &(impu_rec->s->service_profiles[i].public_identities[j]);
-	    if (barring < 0) {
-		//get all records
-		(*impus)[count].s = ptr;
-		memcpy(ptr, impi->public_identity.s, impi->public_identity.len);
-		(*impus)[count].len = impi->public_identity.len;
-		ptr += impi->public_identity.len;
-		count++;
-	    } else {
-		if (impi->barring == barring) {
-		    //add the record to the list
-		    (*impus)[count].s = ptr;
-		    memcpy(ptr, impi->public_identity.s, impi->public_identity.len);
-		    (*impus)[count].len = impi->public_identity.len;
-		    ptr += impi->public_identity.len;
-		    count++;
-		}
-	    }
-	}
+        for (j = 0; j < impu_rec->s->service_profiles[i].public_identities_cnt; j++) {
+            impi = &(impu_rec->s->service_profiles[i].public_identities[j]);
+            if (barring < 0) {
+                //get all records
+                (*impus)[count].s = ptr;
+                memcpy(ptr, impi->public_identity.s, impi->public_identity.len);
+                (*impus)[count].len = impi->public_identity.len;
+                ptr += impi->public_identity.len;
+                count++;
+            } else {
+                if (impi->barring == barring) {
+                    //add the record to the list
+                    (*impus)[count].s = ptr;
+                    memcpy(ptr, impi->public_identity.s, impi->public_identity.len);
+                    (*impus)[count].len = impi->public_identity.len;
+                    ptr += impi->public_identity.len;
+                    count++;
+                }
+            }
+        }
     }
 
     if (ptr != ((char*) *impus + len)) {
-	LM_CRIT("buffer overflow\n");
-	return 1;
+        LM_CRIT("buffer overflow\n");
+        return 1;
     }
 
     unlock_subscription(impu_rec->s);
@@ -681,3 +647,91 @@ int get_impus_from_subscription_as_string(udomain_t* _d, impurecord_t* impu_rec,
     return 0;
 }
 
+/**
+ * @brief Get a subscription from the subscription list based on the IMPI
+ *  NB - does not return with a lock on the subscription but does increment ref count
+ * @param impu string of impu to search for
+ * @param s ims_subscription to be returned if found
+ * @param leave_slot_locked if no subscription is found return with the slot locked (in case we want to add) 
+ * @return 0 on success
+ */
+int get_subscription(str* impi_s, ims_subscription** s, int leave_slot_locked) {
+    int subscription_hash, sl;
+    ims_subscription* ptr;
+
+    subscription_hash = core_hash(impi_s, 0, 0);
+    sl = subscription_hash & (subs_hash_size - 1);
+    lock_subscription_slot(sl);
+    ptr = ims_subscription_list->slot[sl].first;
+    while (ptr) {
+        if ((impi_s->len == ptr->private_identity.len) && (memcmp(impi_s->s, ptr->private_identity.s, impi_s->len) == 0)) {
+            LM_DBG("found an existing subscription for IMPI [%.*s]\n", impi_s->len, impi_s->s);
+            (*s) = ptr;
+            lock_subscription(ptr);
+            ref_subscription_unsafe(ptr);
+            unlock_subscription(ptr);
+            unlock_subscription_slot(sl);
+            return 0;
+        }
+        ptr = ptr->next;
+    }
+    if (!leave_slot_locked)
+        unlock_subscription_slot(sl);
+    return 1;
+}
+
+void add_subscription_unsafe(ims_subscription* s) {
+    int sl;
+    sl = core_hash(&s->private_identity, 0, subs_hash_size);
+    subs_slot_add(&ims_subscription_list->slot[sl], s);
+    s->sl = sl;
+
+}
+
+void add_subscription(ims_subscription* s) {
+    int sl;
+    sl = core_hash(&s->private_identity, 0, subs_hash_size);
+    lock_subscription_slot(sl);
+    add_subscription_unsafe(s);
+    unlock_subscription_slot(sl);
+}
+
+void delete_subscription(ims_subscription* s) {
+    LM_DBG("Deleting subscription %p [%.*s]\n", s, s->private_identity.len, s->private_identity.s);
+    free_ims_subscription_data(s);
+}
+
+void release_subscription(ims_subscription* s) {
+    LM_DBG("Releasing subscription %p [%.*s]\n", s, s->private_identity.len, s->private_identity.s);
+    unref_subscription(s);
+}
+
+/**
+ * @brief update/add subscription
+ * @param s
+ * @return 
+ */
+int update_subscription(ims_subscription* s) {
+    return 0;
+}
+
+void ref_contact_unsafe(ucontact_t* c) {
+    LM_DBG("incrementing ref count on contact [%.*s], was %d\n", c->c.len, c->c.s, c->ref_count);
+    c->ref_count++;
+}
+
+/**
+ * @brief unref contact - assume a lock on the slot is held prior to calling this
+ * @param c
+ */
+void unref_contact_unsafe(ucontact_t* c) {
+    LM_DBG("decrementing ref count on contact [%.*s], was %d\n", c->c.len, c->c.s, c->ref_count);
+    c->ref_count--;
+    if (c->ref_count <= 0) {
+        LM_DBG("contact [%.*s] no longer referenced.... deleting\n", c->c.len, c->c.s);
+        if (c->ref_count < 0) {
+            LM_WARN("reference dropped below zero... this should not happen\n");
+        }
+        delete_ucontact(c);
+    }
+}

modules/ims_usrloc_scscf/udomain.h

@@ -115,7 +115,7 @@ void print_udomain(FILE* _f, udomain_t* _d);
 void mem_timer_udomain(udomain_t* _d);
 
 
-int mem_insert_impurecord(struct udomain* _d, str* public_identity, int reg_state, int barring, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r);
+int mem_insert_impurecord(struct udomain* _d, str* public_identity, str* private_identity, int reg_state, int barring, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r);
 
 
 /*!
@@ -160,31 +160,55 @@ void lock_contact_slot_i(int i);
 void unlock_contact_slot_i(int i);
 void lock_subscription(ims_subscription* s);
 void unlock_subscription(ims_subscription* s);
+void lock_subscription_slot(int i);
+void unlock_subscription_slot(int i);
 
 /* ===== module interface ======= */
 
-
 /*!
- * \brief Create and insert a new record
+ * 
  * \param _d domain to insert the new record
- * \param _aor address of the record
+ * \param public_identity
+ * \param private_identity
+ * \param reg_state
+ * \param barring
+ * \param s
+ * \param ccf1
+ * \param ccf2
+ * \param ecf1
+ * \param ecf2
  * \param _r new created record
- * \return return 0 on success, -1 on failure
+ * \return 
  */
-int insert_impurecord(struct udomain* _d, str* public_identity, int reg_state, int barring,
+int insert_impurecord(struct udomain* _d, str* public_identity, str* private_identity, int reg_state, int barring,
         ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2,
         struct impurecord** _r);
 
 
 /*!
- * \brief Obtain a impurecord pointer if the impurecord exists in domain
+ * \brief Obtain a impurecord pointer if the impurecord exists in domain. You should call this function with a lock on the domain
  * \param _d domain to search the record
  * \param _aor address of record
  * \param _r new created record
  * \return 0 if a record was found, 1 if nothing could be found
  */
+int get_impurecord_unsafe(udomain_t* _d, str* _aor, struct impurecord** _r);
+
+/*!
+ * \brief Obtain a impurecord pointer if the impurecord exists in domain (safe version)
+ * \param _d domain to search the record
+ * \param _aor address of record
+ * \param _r new created record
+ * \return 0 if a record was found, 1 if nothing could be found returns with a lock on the domain
+ */
 int get_impurecord(udomain_t* _d, str* _aor, struct impurecord** _r);
 
+/*!
+ * \brief release the lock on the impurecord - effectively the domain slot
+ * @param _d domain
+ * @param _r impurecord to release (unlock)
+ */
+void release_impurecord(udomain_t* _d, struct impurecord* _r);
 
 /*!
  * \brief Delete a impurecord from domain
@@ -196,11 +220,21 @@ int get_impurecord(udomain_t* _d, str* _aor, struct impurecord** _r);
 int delete_impurecord(udomain_t* _d, str* _aor, struct impurecord* _r);
 
 
-//get all IMPUs as string from a subscription related to an impurecord. apply filter for barring (assumed to be called with lock on impurec)
-//barring-1 get all barred
-//barring-0 get all unbarred
-//barring-(-1) get all records
+/*!get all IMPUs as string from a subscription related to an impurecord. apply filter for barring (assumed to be called with lock on impurec)
+ * barring-1 get all barred
+ * barring-0 get all unbarred
+ * barring-(-1) get all records
+ */
 int get_impus_from_subscription_as_string(udomain_t* _d, impurecord_t* impu_rec, int barring, str** impus, int* num_impus);
 
+int get_subscription(str* impi_s, ims_subscription** s, int leave_slot_locked);
+void add_subscription(ims_subscription* s);
+void add_subscription_unsafe(ims_subscription* s);
+void delete_subscription(ims_subscription* s);
+void release_subscription(ims_subscription* s);
+int update_subscription(ims_subscription* s);
+
+void unref_contact_unsafe(ucontact_t* c);
+void ref_contact_unsafe(ucontact_t* c);
 
 #endif

modules/ims_usrloc_scscf/ul_callback.c

@@ -98,8 +98,6 @@ void destroy_ulcb_list(void)
 	shm_free(ulcb_list);
 }
 
-
-
 /*! \brief 
 	register a callback function 'f' for 'types' mask of events;
 */

modules/ims_usrloc_scscf/ul_mod.c

@@ -146,6 +146,7 @@ static param_export_t params[] = {
 	{"fetch_rows",        	INT_PARAM, &ul_fetch_rows   },
 	{"hash_size",         	INT_PARAM, &ul_hash_size    },
 	{"subs_hash_size",    	INT_PARAM, &subs_hash_size  },
+        {"contacts_hash_size", 	INT_PARAM, &contacts_hash_size  },
 	{"nat_bflag",         	INT_PARAM, &nat_bflag       },
 	{"usrloc_debug_file", 	PARAM_STR, &usrloc_debug_file},
 	{"enable_debug_file", 	INT_PARAM, &usrloc_debug},
@@ -452,10 +453,6 @@ static void timer(unsigned int ticks, void* param) {
         fflush(debug_file);
     }
     
-    if (sync_subscriptions() !=0 ) {
-        LM_ERR("Failed to sync subscriptions\n");
-    }
-
     LM_DBG("Syncing cache\n");
     if (synchronize_all_udomains() != 0) {
         LM_ERR("synchronizing cache failed\n");

modules/ims_usrloc_scscf/ul_scscf_stats.c

@@ -1,13 +1,20 @@
 #include "ul_scscf_stats.h"
+#include "usrloc.h"
+
+extern struct ims_subscription_list* ims_subscription_list;
 
 struct ul_scscf_counters_h ul_scscf_cnts_h;
+enum ul_scscf_info_req { ULSCSF_SUBSCRIPTIONCOUNT };
+
+//static counter_val_t ims_usrloc_scscf_internal_stats(counter_handle_t h, void* what);
 
 counter_def_t ul_scscf_cnt_defs[] = {
-    {&ul_scscf_cnts_h.active_subscriptions,	    "active_subscriptions",	0, 0, 0,			    "number of registered subscribers (IMPIs)"},
-    {&ul_scscf_cnts_h.active_impus,                 "active_impus",             0, 0, 0,			    "number of registered IMPUs"},
-    {&ul_scscf_cnts_h.active_contacts,	    "active_contacts",	0, 0, 0,			    "number of registered contacts"},
-    {0, 0, 0, 0, 0, 0}
-};
+    {&ul_scscf_cnts_h.active_subscriptions, "active_subscriptions", 0, 0, 0, "active_subscriptions"},
+//    {&ul_scscf_cnts_h.active_subscriptions, "active_subscriptions", 0,
+    //            ims_usrloc_scscf_internal_stats, (void*) (long) ULSCSF_SUBSCRIPTIONCOUNT, "number of registered subscribers (IMPIs)"},
+    {&ul_scscf_cnts_h.active_impus, "active_impus", 0, 0, 0, "number of registered IMPUs"},
+    {&ul_scscf_cnts_h.active_contacts, "active_contacts", 0, 0, 0, "number of registered contacts"},
+    {0, 0, 0, 0, 0, 0}};
 
 int ul_scscf_init_counters() {
     if (counter_register_array("ims_usrloc_scscf", ul_scscf_cnt_defs) < 0)
@@ -20,3 +27,16 @@ error:
 void ul_scscf_destroy_counters() {
     
 }
+
+/** helper function for some stats (which are kept internally). to be used in future...
+ */
+//static counter_val_t ims_usrloc_scscf_internal_stats(counter_handle_t h, void* what) {
+//    enum ul_scscf_info_req w;
+//
+//    w = (int) (long) what;
+//    switch (w) {
+//	case ULSCSF_SUBSCRIPTIONCOUNT:
+//	    return ims_subscription_list->subscriptions;
+//    };
+//    return 0;
+//}

modules/ims_usrloc_scscf/ul_scscf_stats.h

@@ -10,10 +10,13 @@
 
 #include "../../counters.h"
 
+extern struct ul_scscf_counters_h ul_scscf_cnts_h;
+
 struct ul_scscf_counters_h {
     counter_handle_t active_subscriptions;
     counter_handle_t active_impus;
     counter_handle_t active_contacts;
+    counter_handle_t expired_contacts;
 };
 
 int ul_scscf_init_counters();

modules/ims_usrloc_scscf/usrloc.c

@@ -78,7 +78,7 @@ int bind_usrloc(usrloc_api_t* api) {
 
 	api->insert_impurecord = insert_impurecord;
 	api->delete_impurecord = delete_impurecord;
-	api->get_impurecord = get_impurecord;
+	api->get_impurecord = get_impurecord;           // get impu - assume no lock on domain yet. returns with lock on doma
 	api->update_impurecord = update_impurecord;
 
 	api->lock_udomain = lock_udomain;
@@ -88,8 +88,12 @@ int bind_usrloc(usrloc_api_t* api) {
 	api->unlock_contact_slot = unlock_contact_slot;
 	api->lock_contact_slot_i = lock_contact_slot_i;
 	api->unlock_contact_slot_i = unlock_contact_slot_i;	
+
         api->lock_subscription = lock_subscription;
         api->unlock_subscription = unlock_subscription;
+        api->ref_subscription = ref_subscription;
+        api->unref_subscription = unref_subscription;
+
 	api->get_all_ucontacts = get_all_ucontacts;
 	api->insert_ucontact = insert_ucontact;
 	api->delete_ucontact = delete_ucontact;
@@ -97,17 +101,19 @@ int bind_usrloc(usrloc_api_t* api) {
 	api->release_ucontact = release_ucontact;
 	api->update_ucontact = update_ucontact;
 	api->expire_ucontact = expire_ucontact;
+
 	api->add_dialog_data_to_contact = add_dialog_data_to_contact;
 	api->remove_dialog_data_from_contact = remove_dialog_data_from_contact;
+
 	api->unlink_contact_from_impu = unlink_contact_from_impu;
 	api->link_contact_to_impu = link_contact_to_impu;
+
 	api->get_subscriber = get_subscriber;
 	api->add_subscriber = add_subscriber;
 	api->external_delete_subscriber = external_delete_subscriber;
 	api->update_subscriber = update_subscriber;
 
 	api->get_impus_from_subscription_as_string = get_impus_from_subscription_as_string;
-	
 	api->get_presentity_from_subscriber_dialog = get_presentity_from_subscriber_dialog;
         
 	api->register_ulcb = register_ulcb;

modules/ims_usrloc_scscf/usrloc.h

@@ -234,7 +234,7 @@ typedef struct {
 typedef struct ims_subscription_s {
     str private_identity; /**< private identity 				*/
     struct hslot_sp* slot; /*!< Collision slot in the hash table array we belong to */
-    unsigned int impu_hash; /**< hash over public_identity */
+    int sl;                 /*!< slot number we belong to */
     int wpsi; /** This is not in the standards
 	 	 	 	 	 	 	 	 	 	 	 	0 normal user or distinct psi inside
 	 	 	 	 	 	 	 	 	 	 	 	1 wildcarded psi
@@ -295,7 +295,7 @@ typedef struct contact_dialog_data {
 typedef struct ucontact {
     gen_lock_t *lock;           /**< we have to lock the contact as it is shared by many impu structs and has reference conting	*/
     struct contact_hslot* slot; /*!< Collision slot in the hash table array we belong to */
-    unsigned int contact_hash; 			/*!< Hash over contact */
+    unsigned int sl; 			/*!< Hash slot number we belong to */
     int ref_count;
     contact_state_t state;
     str domain; /*!< Pointer to domain name (NULL terminated) */
@@ -372,13 +372,26 @@ static inline char* get_impu_regstate_as_string(enum pi_reg_states reg_state) {
     }
 }
 
+typedef struct _contact_ptr {
+    ucontact_t* contact;
+    struct contact_ptr* prev;
+    struct contact_ptr* next;
+} contact_ptr;
+
+typedef struct _contact_ptr_list {
+    int n;
+    gen_lock_t* lock;
+    struct contact_ptr* first; 
+} contact_ptr_list;
+
 /*! \brief
  * Basic hash table element
  */
 typedef struct impurecord {
-    str* domain; 					/*!< Pointer to domain we belong to (null terminated string) */
-    int is_primary;					/*!< first IMPU (in implicit set this is the one that will trigger a SAR, if no implicit set - we should still be safe with first) */
+    str* domain; 				/*!< Pointer to domain we belong to (null terminated string) */
+    int is_primary;				/*!< first IMPU (in implicit set this is the one that will trigger a SAR, if no implicit set - we should still be safe with first) */
     str public_identity; 			/*!< Address of record */
+    str private_identity;                       /*!< Subscription to which we be long */
     unsigned int aorhash; 			/*!< Hash over address of record */
     int barring;
     enum pi_reg_states reg_state;
@@ -416,7 +429,7 @@ typedef struct ims_subscription_list {
     int subscriptions;      /* total number of subscriptions in storage */
 }ims_subscription_list_t;
 
-typedef int (*insert_impurecord_t)(struct udomain* _d, str* public_identity, int reg_state, int barring,
+typedef int (*insert_impurecord_t)(struct udomain* _d, str* public_identity, str* private_identity, int reg_state, int barring,
         ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2,
         struct impurecord** _r);
 
@@ -424,7 +437,7 @@ typedef int (*get_impurecord_t)(struct udomain* _d, str* _aor, struct impurecord
 
 typedef int (*delete_impurecord_t)(struct udomain* _d, str* _aor, struct impurecord* _r);
 
-typedef int (*update_impurecord_t)(struct udomain* _d, str* public_identity, int reg_state, int send_sar_on_delete, int barring, int is_primary, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r);
+typedef int (*update_impurecord_t)(struct udomain* _d, str* public_identity, impurecord_t* impu_rec, int reg_state, int send_sar_on_delete, int barring, int is_primary, ims_subscription** s, str* ccf1, str* ccf2, str* ecf1, str* ecf2, struct impurecord** _r);
 
 typedef void (*lock_contact_slot_t)(str* contact_uri);
 
@@ -437,6 +450,8 @@ typedef void (*unlock_contact_slot_i_t)(int sl);
 typedef void (*lock_subscription_t)(ims_subscription* s);
 
 typedef void (*unlock_subscription_t)(ims_subscription* s);
+typedef void (*unref_subscription_t) (ims_subscription* s);
+typedef void (*ref_subscription_t) (ims_subscription* s);
 
 typedef int (*update_ucontact_t)(struct impurecord* _r, struct ucontact* _c, struct ucontact_info* _ci);
 
@@ -504,6 +519,8 @@ typedef struct usrloc_api {
     unlock_contact_slot_i_t unlock_contact_slot_i;
     lock_subscription_t lock_subscription;
     unlock_subscription_t unlock_subscription;
+    unref_subscription_t unref_subscription;
+    ref_subscription_t ref_subscription;
     
     insert_ucontact_t insert_ucontact;
     delete_ucontact_t delete_ucontact;

modules/ims_usrloc_scscf/usrloc_db.c

@@ -784,8 +784,8 @@ int preload_udomain(db1_con_t* _c, udomain_t* _d) {
 
 	    /* insert impu into memory */
 	    lock_udomain(_d, &impu);
-	    if (get_impurecord(_d, &impu, &impurecord) != 0) {
-		if (mem_insert_impurecord(_d, &impu, reg_state, barring,
+	    if (get_impurecord_unsafe(_d, &impu, &impurecord) != 0) {
+		if (mem_insert_impurecord(_d, &impu, &subscription->private_identity,reg_state, barring,
 			&subscription, &ccf1, &ccf2, &ecf1, &ecf2, &impurecord)
 			!= 0) {
 		    LM_ERR("Unable to insert IMPU into memory [%.*s]\n", impu.len, impu.s);