ldap/servers/plugins/roles/roles_cache.c | 4 - ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 2 ldap/servers/slapd/backend.c | 81 +++++++++++++++++++------------ ldap/servers/slapd/backend_manager.c | 19 ++++--- ldap/servers/slapd/slap.h | 8 ++- 5 files changed, 71 insertions(+), 43 deletions(-)
New commits: commit 1b6f39cbb800b9c70abff1d025fecdd812a5b7b6 Author: Mark Reynolds mreynolds@redhat.com Date: Tue Dec 11 14:20:38 2012 -0500
Ticket 509 - lock-free access to be->be_suffixlock
Bug Description: This is an amendment to the first patch which was not safe because of the reallocation of the suffix list
Fix Description: Use a linked list instead of a pointer array, as the array needs to be reallocated which each add.
https://fedorahosted.org/389/ticket/509
Reviewed by: Noriko, and Rich (Thanks!)
diff --git a/ldap/servers/plugins/roles/roles_cache.c b/ldap/servers/plugins/roles/roles_cache.c index 83d3292..89acc59 100644 --- a/ldap/servers/plugins/roles/roles_cache.c +++ b/ldap/servers/plugins/roles/roles_cache.c @@ -929,7 +929,7 @@ if ( e != NULL ) } } #endif - Slapi_DN *top_suffix = roles_cache_get_top_suffix(*(be->be_suffix)); + Slapi_DN *top_suffix = roles_cache_get_top_suffix((Slapi_DN *)slapi_be_getsuffix(be, 0));
if ( top_suffix != NULL ) { @@ -1714,7 +1714,7 @@ static int roles_cache_find_roles_in_suffix(Slapi_DN *target_entry_dn, roles_cac backend = slapi_mapping_tree_find_backend_for_sdn(target_entry_dn); if ( (backend != NULL) && !slapi_be_is_flag_set(backend,SLAPI_BE_FLAG_REMOTE_DATA) ) { - Slapi_DN *suffix = roles_cache_get_top_suffix(*(backend->be_suffix)); + Slapi_DN *suffix = roles_cache_get_top_suffix((Slapi_DN *)slapi_be_getsuffix(backend, 0)); roles_cache_def *current_role = roles_list;
/* Go through all the roles list and trigger the associated structure */ diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c index 578701a..957ebf5 100644 --- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c +++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c @@ -796,7 +796,7 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) /* for each subtree spec... */ for (i = 0; include[i]; i++) { IDList *idl = NULL; - const char *suffix = slapi_sdn_get_ndn(*be->be_suffix); + const char *suffix = slapi_sdn_get_ndn(slapi_be_getsuffix(be, 0)); char *parentdn = slapi_ch_strdup(suffix); char *nextdn = NULL; int matched = 0; diff --git a/ldap/servers/slapd/backend.c b/ldap/servers/slapd/backend.c index e8d71c3..f964856 100644 --- a/ldap/servers/slapd/backend.c +++ b/ldap/servers/slapd/backend.c @@ -48,7 +48,7 @@ void be_init( Slapi_Backend *be, const char *type, const char *name, int isprivate, int logchanges, int sizelimit, int timelimit ) { slapdFrontendConfig_t *fecfg; - be->be_suffix = NULL; + be->be_suffixlist = NULL; be->be_suffixlock = PR_NewLock(); be->be_suffixcounter = slapi_counter_new(); /* e.g. dn: cn=config,cn=NetscapeRoot,cn=ldbm database,cn=plugins,cn=config */ @@ -117,12 +117,16 @@ be_done(Slapi_Backend *be) { int i; int count = slapi_counter_get_value(be->be_suffixcounter); + struct suffixlist *list, *next;
- for(i=0; i < count; i++) + list = be->be_suffixlist; + for(i=0; i < count && list; i++) { - slapi_sdn_free(&be->be_suffix[i]); + next = list->next; + slapi_sdn_free(&list->be_suffix); + slapi_ch_free((void **)&list); + list = next; } - slapi_ch_free((void**)&be->be_suffix); slapi_ch_free((void **)&be->be_basedn); slapi_ch_free((void **)&be->be_configdn); slapi_ch_free((void **)&be->be_monitordn); @@ -168,20 +172,23 @@ slapi_be_get_readonly(Slapi_Backend *be) int slapi_be_issuffix( const Slapi_Backend *be, const Slapi_DN *suffix ) { + struct suffixlist *list; int r= 0; /* this backend is no longer valid */ if (be->be_state != BE_STATE_DELETED) { - int i, count; - count = slapi_counter_get_value(be->be_suffixcounter); - for ( i = 0; be->be_suffix != NULL && i < count; i++ ) - { - if ( slapi_sdn_compare( be->be_suffix[i], suffix ) == 0) - { - r= 1; + int i = 0, count; + + count = slapi_counter_get_value(be->be_suffixcounter); + list = be->be_suffixlist; + while(list && i < count){ + if ( slapi_sdn_compare( list->be_suffix, suffix ) == 0){ + r = 1; break; - } - } + } + i++; + list = list->next; + } } return r; } @@ -197,20 +204,25 @@ be_addsuffix(Slapi_Backend *be,const Slapi_DN *suffix) { if (be->be_state != BE_STATE_DELETED) { - int count; + struct suffixlist *new_suffix, *list; + + new_suffix = (struct suffixlist *)slapi_ch_malloc(sizeof(struct suffixlist *)); + new_suffix->be_suffix = slapi_sdn_dup(suffix); + new_suffix->next = NULL;
PR_Lock(be->be_suffixlock); - count = slapi_counter_get_value(be->be_suffixcounter); - if(be->be_suffix==NULL) - { - be->be_suffix= (Slapi_DN **)slapi_ch_malloc(sizeof(Slapi_DN *)); - } - else - { - be->be_suffix= (Slapi_DN **)slapi_ch_realloc((char*)be->be_suffix,(count+1)*sizeof(Slapi_DN *)); + + if(be->be_suffixlist == NULL){ + be->be_suffixlist = new_suffix; + } else { + list = be->be_suffixlist; + while(list->next != NULL){ + list = list->next; + } + list->next = new_suffix; } - be->be_suffix[count]= slapi_sdn_dup(suffix); slapi_counter_increment(be->be_suffixcounter); + PR_Unlock(be->be_suffixlock); } } @@ -221,25 +233,32 @@ void slapi_be_addsuffix(Slapi_Backend *be,const Slapi_DN *suffix) }
/* - * The caller may use the returned pointer without holding the - * be_suffixlock since we never remove suffixes from the array. * The Slapi_DN pointer will always be valid even though the array * itself may be changing due to the addition of a suffix. */ const Slapi_DN * slapi_be_getsuffix(Slapi_Backend *be,int n) { - Slapi_DN *sdn = NULL; + struct suffixlist *list;
- if(NULL == be) - return NULL; + if(NULL == be) + return NULL;
if(be->be_state != BE_STATE_DELETED) { - if (be->be_suffix !=NULL && n < slapi_counter_get_value(be->be_suffixcounter)) { - sdn = be->be_suffix[n]; + if (be->be_suffixlist !=NULL && n < slapi_counter_get_value(be->be_suffixcounter)) { + int i = 0; + + list = be->be_suffixlist; + while(list != NULL && i <= n){ + if(i == n){ + return list->be_suffix; + } + list = list->next; + i++; + } } } - return sdn; + return NULL; }
const char * diff --git a/ldap/servers/slapd/backend_manager.c b/ldap/servers/slapd/backend_manager.c index f27f713..a8921c4 100644 --- a/ldap/servers/slapd/backend_manager.c +++ b/ldap/servers/slapd/backend_manager.c @@ -462,6 +462,7 @@ slapi_lookup_instance_name_by_suffix(char *suffix, char ***suffixes, char ***instances, int isexact) { Slapi_Backend *be = NULL; + struct suffixlist *list; char *cookie = NULL; const char *thisdn; int thisdnlen; @@ -480,23 +481,27 @@ slapi_lookup_instance_name_by_suffix(char *suffix, cookie = NULL; be = slapi_get_first_backend (&cookie); while (be) { - if (NULL == be->be_suffix) { + if (NULL == be->be_suffixlist) { be = (backend *)slapi_get_next_backend (cookie); continue; } count = slapi_counter_get_value(be->be_suffixcounter); - for (i = 0; be->be_suffix && i < count; i++) { - thisdn = slapi_sdn_get_ndn(be->be_suffix[i]); - thisdnlen = slapi_sdn_get_ndn_len(be->be_suffix[i]); - if (isexact?suffixlen!=thisdnlen:suffixlen>thisdnlen) + list = be->be_suffixlist; + for (i = 0; list && i < count; i++) { + thisdn = slapi_sdn_get_ndn(list->be_suffix); + thisdnlen = slapi_sdn_get_ndn_len(list->be_suffix); + if (isexact?suffixlen!=thisdnlen:suffixlen>thisdnlen){ + list = list->next; continue; + } if (isexact?(!slapi_UTF8CASECMP(suffix, (char *)thisdn)): - (!slapi_UTF8CASECMP(suffix, - (char *)thisdn+thisdnlen-suffixlen))) { + (!slapi_UTF8CASECMP(suffix, (char *)thisdn+thisdnlen-suffixlen))) + { charray_add(instances, slapi_ch_strdup(be->be_name)); if (suffixes) charray_add(suffixes, slapi_ch_strdup(thisdn)); } + list = list->next; } be = (backend *)slapi_get_next_backend (cookie); } diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index dee016a..8797c04 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1158,12 +1158,16 @@ struct slapdplugin { } plg_un; };
+struct suffixlist { + Slapi_DN *be_suffix; + struct suffixlist *next; +}; + /* * represents a "database" */ - typedef struct backend { - Slapi_DN **be_suffix; /* the DN suffixes of data in this backend */ + struct suffixlist *be_suffixlist; /* linked list of DN suffixes in this backend */ PRLock *be_suffixlock; Slapi_Counter *be_suffixcounter; char *be_basedn; /* The base dn for the config & monitor dns */