dirsrvtests/tickets/ticket47973_test.py | 235 +++++++++++++++++ ldap/servers/plugins/schema_reload/schema_reload.c | 7 ldap/servers/slapd/attr.c | 8 ldap/servers/slapd/attrsyntax.c | 282 +++++++++++++++------ ldap/servers/slapd/opshared.c | 2 ldap/servers/slapd/proto-slap.h | 11 ldap/servers/slapd/schema.c | 51 ++- 7 files changed, 486 insertions(+), 110 deletions(-)
New commits: commit 2da0bd62830b5240d110cc4a52a93da07371511e Author: Mark Reynolds mreynolds@redhat.com Date: Thu Jan 8 15:43:43 2015 -0500
Ticket 47973 - During schema reload sometimes the search returns no results
Bug Description: During a schema reload operation the search of an existing entry may randomly report no results. This is because during the schema reload all existing attribute syntaxes are removed, and there is a small window where ldap operations can be performed before the new schema is loaded. During this window, attribute values are normalized to NULL which causes operation to unexpectedly fail.
Fix Description: Instead of wiping out the existing attribute syntax hastables in the middle of the task, instead load the new schema into temporary hash tables. Once the reload is complete, then take the asi write lock, remove the old hash tables, and swap in the new ones.
https://fedorahosted.org/389/ticket/47973
valgrind: PASSED
Reviewed by: nhosoi(Thanks!)
(cherry picked from commit e0c78e1c1db351ea5cf835cdb55437b1ea26a3a6)
diff --git a/dirsrvtests/tickets/ticket47973_test.py b/dirsrvtests/tickets/ticket47973_test.py new file mode 100644 index 0000000..11b1ac8 --- /dev/null +++ b/dirsrvtests/tickets/ticket47973_test.py @@ -0,0 +1,235 @@ +import os +import sys +import time +import ldap +import ldap.sasl +import logging +import socket +import pytest +from lib389 import DirSrv, Entry, tools, tasks +from lib389.tools import DirSrvTools +from lib389._constants import * +from lib389.properties import * +from lib389.tasks import * +from constants import * + +log = logging.getLogger(__name__) + +installation_prefix = None + +USER_DN = 'uid=user1,%s' % (DEFAULT_SUFFIX) +SCHEMA_RELOAD_COUNT = 10 + + +class TopologyStandalone(object): + def __init__(self, standalone): + standalone.open() + self.standalone = standalone + + +@pytest.fixture(scope="module") +def topology(request): + ''' + This fixture is used to standalone topology for the 'module'. + At the beginning, It may exists a standalone instance. + It may also exists a backup for the standalone instance. + + Principle: + If standalone instance exists: + restart it + If backup of standalone exists: + create/rebind to standalone + + restore standalone instance from backup + else: + Cleanup everything + remove instance + remove backup + Create instance + Create backup + ''' + global installation_prefix + + if installation_prefix: + args_instance[SER_DEPLOYED_DIR] = installation_prefix + + standalone = DirSrv(verbose=False) + + # Args for the standalone instance + args_instance[SER_HOST] = HOST_STANDALONE + args_instance[SER_PORT] = PORT_STANDALONE + args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE + args_standalone = args_instance.copy() + standalone.allocate(args_standalone) + + # Get the status of the backups + backup_standalone = standalone.checkBackupFS() + + # Get the status of the instance and restart it if it exists + instance_standalone = standalone.exists() + if instance_standalone: + # assuming the instance is already stopped, just wait 5 sec max + standalone.stop(timeout=5) + standalone.start(timeout=10) + + if backup_standalone: + # The backup exist, assuming it is correct + # we just re-init the instance with it + if not instance_standalone: + standalone.create() + # Used to retrieve configuration information (dbdir, confdir...) + standalone.open() + + # restore standalone instance from backup + standalone.stop(timeout=10) + standalone.restoreFS(backup_standalone) + standalone.start(timeout=10) + + else: + # We should be here only in two conditions + # - This is the first time a test involve standalone instance + # - Something weird happened (instance/backup destroyed) + # so we discard everything and recreate all + + # Remove the backup. So even if we have a specific backup file + # (e.g backup_standalone) we clear backup that an instance may have created + if backup_standalone: + standalone.clearBackupFS() + + # Remove the instance + if instance_standalone: + standalone.delete() + + # Create the instance + standalone.create() + + # Used to retrieve configuration information (dbdir, confdir...) + standalone.open() + + # Time to create the backups + standalone.stop(timeout=10) + standalone.backupfile = standalone.backupFS() + standalone.start(timeout=10) + + # clear the tmp directory + standalone.clearTmpDir(__file__) + + # + # Here we have standalone instance up and running + # Either coming from a backup recovery + # or from a fresh (re)init + # Time to return the topology + return TopologyStandalone(standalone) + + +def task_complete(conn, task_dn): + finished = False + + try: + task_entry = conn.search_s(task_dn, ldap.SCOPE_BASE, 'objectclass=*') + if not task_entry: + log.fatal('wait_for_task: Search failed to find task: ' + task_dn) + assert False + if task_entry[0].hasAttr('nstaskexitcode'): + # task is done + finished = True + except ldap.LDAPError, e: + log.fatal('wait_for_task: Search failed: ' + e.message['desc']) + assert False + + return finished + + +def test_ticket47973(topology): + """ + During the schema reload task there is a small window where the new schema is not loaded + into the asi hashtables - this results in searches not returning entries. + """ + + log.info('Testing Ticket 47973 - Test the searches still work as expected during schema reload tasks') + + # + # Add a user + # + try: + topology.standalone.add_s(Entry((USER_DN, { + 'objectclass': 'top extensibleObject'.split(), + 'uid': 'user1' + }))) + except ldap.LDAPError, e: + log.error('Failed to add user1: error ' + e.message['desc']) + assert False + + # + # Run a series of schema_reload tasks while searching for our user. Since + # this is a race condition, run it several times. + # + task_count = 0 + while task_count < SCHEMA_RELOAD_COUNT: + # + # Add a schema reload task + # + + TASK_DN = 'cn=task-' + str(task_count) + ',cn=schema reload task, cn=tasks, cn=config' + try: + topology.standalone.add_s(Entry((TASK_DN, { + 'objectclass': 'top extensibleObject'.split(), + 'cn': 'task-' + str(task_count) + }))) + except ldap.LDAPError, e: + log.error('Failed to add task entry: error ' + e.message['desc']) + assert False + + # + # While we wait for the task to complete keep searching for our user + # + search_count = 0 + while search_count < 100: + # + # Now check the user is still being returned + # + try: + entries = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + '(uid=user1)') + if not entries or not entries[0]: + log.fatal('User was not returned from search!') + assert False + except ldap.LDAPError, e: + log.fatal('Unable to search for entry %s: error %s' % (USER_DN, e.message['desc'])) + assert False + + # + # Check if task is complete + # + if task_complete(topology.standalone, TASK_DN): + break + + search_count += 1 + + task_count += 1 + + # If we got here the test passed + log.info('Test PASSED') + + +def test_ticket47973_final(topology): + topology.standalone.stop(timeout=10) + + +def run_isolated(): + ''' + run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..) + To run isolated without py.test, you need to + - edit this file and comment '@pytest.fixture' line before 'topology' function. + - set the installation prefix + - run this program + ''' + global installation_prefix + installation_prefix = None + + topo = topology(True) + test_ticket47973(topo) + +if __name__ == '__main__': + run_isolated() \ No newline at end of file diff --git a/ldap/servers/plugins/schema_reload/schema_reload.c b/ldap/servers/plugins/schema_reload/schema_reload.c index b1a5bb8..bb16d08 100644 --- a/ldap/servers/plugins/schema_reload/schema_reload.c +++ b/ldap/servers/plugins/schema_reload/schema_reload.c @@ -187,13 +187,6 @@ schemareload_thread(void *arg) slapi_task_log_notice(task, "Schema reload task finished."); slapi_task_log_status(task, "Schema reload task finished."); slapi_log_error(SLAPI_LOG_FATAL, "schemareload", "Schema reload task finished.\n"); - - slapi_log_error(SLAPI_LOG_FATAL, "schemareload", - "Register internal schema.\n"); - rv = slapi_reload_internal_attr_syntax(); - slapi_log_error(SLAPI_LOG_FATAL, "schemareload", - "Register internal schema finished.\n"); - } else { slapi_task_log_notice(task, "Schema reload task failed."); slapi_task_log_status(task, "Schema reload task failed."); diff --git a/ldap/servers/slapd/attr.c b/ldap/servers/slapd/attr.c index a4cf6cb..f19e7e6 100644 --- a/ldap/servers/slapd/attr.c +++ b/ldap/servers/slapd/attr.c @@ -245,8 +245,8 @@ slapi_attr_types_equivalent(const char *t1, const char *t2) return 0; }
- asi1 = attr_syntax_get_by_name(t1); - asi2 = attr_syntax_get_by_name(t2); + asi1 = attr_syntax_get_by_name(t1, 0); + asi2 = attr_syntax_get_by_name(t2, 0); if (NULL != asi1) { if (NULL != asi2) { /* Both found - compare normalized names */ @@ -347,7 +347,7 @@ slapi_attr_init_locking_optional(Slapi_Attr *a, const char *type, PRBool use_loc { basetype = tmp; /* basetype was malloc'd */ } - asi = attr_syntax_get_by_name_locking_optional(basetype, use_lock); + asi = attr_syntax_get_by_name_locking_optional(basetype, use_lock, 0); } if(NULL == asi) { @@ -358,7 +358,7 @@ slapi_attr_init_locking_optional(Slapi_Attr *a, const char *type, PRBool use_loc * attribute type that has that syntax. */ asi = attr_syntax_get_by_name_locking_optional( - ATTR_WITH_OCTETSTRING_SYNTAX, use_lock); + ATTR_WITH_OCTETSTRING_SYNTAX, use_lock, 0); } else { diff --git a/ldap/servers/slapd/attrsyntax.c b/ldap/servers/slapd/attrsyntax.c index 885d02a..d93a807 100644 --- a/ldap/servers/slapd/attrsyntax.c +++ b/ldap/servers/slapd/attrsyntax.c @@ -70,8 +70,15 @@ static asyntaxinfo *global_at = NULL; static PLHashTable *name2asi = NULL; /* read/write lock to protect table */ static Slapi_RWLock *name2asi_lock = NULL; -static int asi_locking = 1;
+/* + * For the schema reload task, we need to use separate temporary hashtables & linked lists + */ +static PLHashTable *oid2asi_tmp = NULL; +static PLHashTable *name2asi_tmp = NULL; +static asyntaxinfo *global_at_tmp = NULL; + +static int asi_locking = 1; #define AS_LOCK_READ(l) if (asi_locking) { slapi_rwlock_rdlock(l); } #define AS_LOCK_WRITE(l) if (asi_locking) { slapi_rwlock_wrlock(l); } #define AS_UNLOCK_READ(l) if (asi_locking) { slapi_rwlock_unlock(l); } @@ -82,10 +89,11 @@ static struct asyntaxinfo *default_asi = NULL;
static void *attr_syntax_get_plugin_by_name_with_default( const char *type ); static void attr_syntax_delete_no_lock( struct asyntaxinfo *asip, - PRBool remove_from_oid_table ); + PRBool remove_from_oid_table, PRUint32 schema_flags ); static struct asyntaxinfo *attr_syntax_get_by_oid_locking_optional( const - char *oid, PRBool use_lock); + char *oid, PRBool use_lock, PRUint32 schema_flags); static void attr_syntax_insert( struct asyntaxinfo *asip ); +static void attr_syntax_insert_tmp( struct asyntaxinfo *asip ); static void attr_syntax_remove( struct asyntaxinfo *asip );
#ifdef ATTR_LDAP_DEBUG @@ -210,8 +218,6 @@ attr_syntax_check_oids() void attr_syntax_free( struct asyntaxinfo *a ) { - PR_ASSERT( a->asi_refcnt == 0 ); - cool_charray_free( a->asi_aliases ); slapi_ch_free_string(&a->asi_name ); slapi_ch_free_string(&a->asi_desc ); @@ -241,9 +247,9 @@ attr_syntax_new() * be returned by calling to attr_syntax_return(). */ struct asyntaxinfo * -attr_syntax_get_by_oid(const char *oid) +attr_syntax_get_by_oid(const char *oid, PRUint32 schema_flags) { - return attr_syntax_get_by_oid_locking_optional( oid, PR_TRUE); + return attr_syntax_get_by_oid_locking_optional( oid, PR_TRUE, schema_flags); }
@@ -256,15 +262,30 @@ attr_syntax_get_by_oid(const char *oid) * same use_lock parameter. */ static struct asyntaxinfo * -attr_syntax_get_by_oid_locking_optional( const char *oid, PRBool use_lock ) +attr_syntax_get_by_oid_locking_optional( const char *oid, PRBool use_lock, PRUint32 schema_flags ) { struct asyntaxinfo *asi = 0; - if (oid2asi) + PLHashTable *ht = oid2asi; + int using_tmp_ht = 0; + + if (schema_flags & DSE_SCHEMA_LOCKED){ + ht = oid2asi_tmp; + using_tmp_ht = 1; + use_lock = 0; + } + if (ht) { if ( use_lock ) { AS_LOCK_READ(oid2asi_lock); } - asi = (struct asyntaxinfo *)PL_HashTableLookup_const(oid2asi, oid); + if (!using_tmp_ht){ + /* + * The oid2asi pointer could have been rewritten by the schema_reload task + * while waiting on the lock, so grab it again. + */ + ht = oid2asi; + } + asi = (struct asyntaxinfo *)PL_HashTableLookup_const(ht, oid); if (asi) { PR_AtomicIncrement( &asi->asi_refcnt ); @@ -285,18 +306,22 @@ attr_syntax_get_by_oid_locking_optional( const char *oid, PRBool use_lock ) * to worry about resource contention. */ static void -attr_syntax_add_by_oid(const char *oid, struct asyntaxinfo *a, int lock) +attr_syntax_add_by_oid(const char *oid, struct asyntaxinfo *a, PRUint32 schema_flags, int lock) { if (0 != attr_syntax_init()) return;
- if (lock) { - AS_LOCK_WRITE(oid2asi_lock); - } + if(schema_flags & DSE_SCHEMA_LOCKED){ + PL_HashTableAdd(oid2asi_tmp, oid, a); + } else { + if (lock) { + AS_LOCK_WRITE(oid2asi_lock); + }
- PL_HashTableAdd(oid2asi, oid, a); + PL_HashTableAdd(oid2asi, oid, a);
- if (lock) { - AS_UNLOCK_WRITE(oid2asi_lock); + if (lock) { + AS_UNLOCK_WRITE(oid2asi_lock); + } } }
@@ -309,18 +334,18 @@ attr_syntax_add_by_oid(const char *oid, struct asyntaxinfo *a, int lock) * be returned by calling to attr_syntax_return(). */ struct asyntaxinfo * -attr_syntax_get_by_name(const char *name) +attr_syntax_get_by_name(const char *name, PRUint32 schema_flags) { - return attr_syntax_get_by_name_locking_optional(name, PR_TRUE); + return attr_syntax_get_by_name_locking_optional(name, PR_TRUE, schema_flags); }
struct asyntaxinfo * attr_syntax_get_by_name_with_default(const char *name) { struct asyntaxinfo *asi = NULL; - asi = attr_syntax_get_by_name_locking_optional(name, PR_TRUE); + asi = attr_syntax_get_by_name_locking_optional(name, PR_TRUE, 0); if (asi == NULL) - asi = attr_syntax_get_by_name(ATTR_WITH_OCTETSTRING_SYNTAX); + asi = attr_syntax_get_by_name(ATTR_WITH_OCTETSTRING_SYNTAX, 0); if ( asi == NULL ) asi = default_asi; return asi; @@ -335,15 +360,30 @@ struct asyntaxinfo *asi = NULL; * same use_lock parameter. */ struct asyntaxinfo * -attr_syntax_get_by_name_locking_optional(const char *name, PRBool use_lock) +attr_syntax_get_by_name_locking_optional(const char *name, PRBool use_lock, PRUint32 schema_flags) { struct asyntaxinfo *asi = 0; - if (name2asi) + PLHashTable *ht = name2asi; + int using_tmp_ht = 0; + + if (schema_flags & DSE_SCHEMA_LOCKED){ + ht = name2asi_tmp; + using_tmp_ht = 1; + use_lock = 0; + } + if (ht) { if ( use_lock ) { AS_LOCK_READ(name2asi_lock); } - asi = (struct asyntaxinfo *)PL_HashTableLookup_const(name2asi, name); + if(!using_tmp_ht){ + /* + * The name2asi pointer could have been rewritten by the schema_reload task + * while waiting on the lock, so grab it again. + */ + ht = name2asi; + } + asi = (struct asyntaxinfo *)PL_HashTableLookup_const(ht, name); if ( NULL != asi ) { PR_AtomicIncrement( &asi->asi_refcnt ); } @@ -352,7 +392,7 @@ attr_syntax_get_by_name_locking_optional(const char *name, PRBool use_lock) } } if (!asi) /* given name may be an OID */ - asi = attr_syntax_get_by_oid_locking_optional(name, use_lock); + asi = attr_syntax_get_by_oid_locking_optional(name, use_lock, schema_flags);
return asi; } @@ -416,29 +456,43 @@ attr_syntax_return_locking_optional(struct asyntaxinfo *asi, PRBool use_lock) * to worry about resource contention. */ static void -attr_syntax_add_by_name(struct asyntaxinfo *a, int lock) +attr_syntax_add_by_name(struct asyntaxinfo *a, PRUint32 schema_flags, int lock) { if (0 != attr_syntax_init()) return;
- if (lock) { - AS_LOCK_WRITE(name2asi_lock); - } + if (schema_flags & DSE_SCHEMA_LOCKED ){ + /* insert the attr into the temp global linked list */ + attr_syntax_insert_tmp(a); + + PL_HashTableAdd(name2asi_tmp, a->asi_name, a); + if ( a->asi_aliases != NULL ) { + int i; + + for ( i = 0; a->asi_aliases[i] != NULL; ++i ) { + PL_HashTableAdd(name2asi_tmp, a->asi_aliases[i], a); + } + } + } else { + if (lock) { + AS_LOCK_WRITE(name2asi_lock); + } + /* insert the attr into the global linked list */ + attr_syntax_insert(a);
- /* insert the attr into the global linked list */ - attr_syntax_insert(a); + PL_HashTableAdd(name2asi, a->asi_name, a); + if ( a->asi_aliases != NULL ) { + int i;
- PL_HashTableAdd(name2asi, a->asi_name, a); - if ( a->asi_aliases != NULL ) { - int i; + for ( i = 0; a->asi_aliases[i] != NULL; ++i ) { + PL_HashTableAdd(name2asi, a->asi_aliases[i], a); + } + }
- for ( i = 0; a->asi_aliases[i] != NULL; ++i ) { - PL_HashTableAdd(name2asi, a->asi_aliases[i], a); + if (lock) { + AS_UNLOCK_WRITE(name2asi_lock); } }
- if (lock) { - AS_UNLOCK_WRITE(name2asi_lock); - } }
@@ -447,7 +501,7 @@ attr_syntax_add_by_name(struct asyntaxinfo *a, int lock) * and oids. */ void -attr_syntax_delete( struct asyntaxinfo *asi ) +attr_syntax_delete( struct asyntaxinfo *asi, PRUint32 schema_flags ) { PR_ASSERT( asi );
@@ -455,7 +509,7 @@ attr_syntax_delete( struct asyntaxinfo *asi ) AS_LOCK_WRITE(oid2asi_lock); AS_LOCK_WRITE(name2asi_lock);
- attr_syntax_delete_no_lock( asi, PR_TRUE ); + attr_syntax_delete_no_lock( asi, PR_TRUE, schema_flags );
AS_UNLOCK_WRITE(name2asi_lock); AS_UNLOCK_WRITE(oid2asi_lock); @@ -469,19 +523,34 @@ attr_syntax_delete( struct asyntaxinfo *asi ) */ static void attr_syntax_delete_no_lock( struct asyntaxinfo *asi, - PRBool remove_from_oidtable ) + PRBool remove_from_oidtable, PRUint32 schema_flags ) { - int i; + PLHashTable *ht = NULL; + int using_tmp_ht = 0; + int i;
+ if (schema_flags & DSE_SCHEMA_LOCKED){ + using_tmp_ht = 1; + } if (oid2asi && remove_from_oidtable ) { - PL_HashTableRemove(oid2asi, asi->asi_oid); + if (using_tmp_ht){ + ht = oid2asi_tmp; + } else { + ht = oid2asi; + } + PL_HashTableRemove(ht, asi->asi_oid); }
if(name2asi) { - PL_HashTableRemove(name2asi, asi->asi_name); + if (using_tmp_ht){ + ht = name2asi_tmp; + } else { + ht = name2asi; + } + PL_HashTableRemove(ht, asi->asi_name); if ( asi->asi_aliases != NULL ) { for ( i = 0; asi->asi_aliases[i] != NULL; ++i ) { - PL_HashTableRemove(name2asi, asi->asi_aliases[i]); + PL_HashTableRemove(ht, asi->asi_aliases[i]); } } if ( asi->asi_refcnt > 0 ) { @@ -511,7 +580,7 @@ slapi_attr_syntax_normalize( const char *s ) struct asyntaxinfo *asi = NULL; char *r = NULL;
- if((asi=attr_syntax_get_by_name(s)) != NULL ) { + if((asi=attr_syntax_get_by_name(s, 0)) != NULL ) { r = slapi_ch_strdup(asi->asi_name); attr_syntax_return( asi ); } @@ -535,7 +604,7 @@ slapi_attr_syntax_normalize_ext( char *s, int flags ) struct asyntaxinfo *asi = NULL; char *r = NULL;
- if((asi=attr_syntax_get_by_name(s)) != NULL ) { + if((asi=attr_syntax_get_by_name(s, flags)) != NULL ) { r = slapi_ch_strdup(asi->asi_name); attr_syntax_return( asi ); } @@ -569,7 +638,7 @@ attr_syntax_exists(const char *attr_name) check_attr_name = (char *)attr_name; }
- asi = attr_syntax_get_by_name(check_attr_name); + asi = attr_syntax_get_by_name(check_attr_name, 0); attr_syntax_return( asi );
if (free_attr) { @@ -742,13 +811,13 @@ attr_syntax_get_plugin_by_name_with_default( const char *type ) /* * first we look for this attribute type explictly */ - if ( (asi = attr_syntax_get_by_name(type)) == NULL ) { + if ( (asi = attr_syntax_get_by_name(type, 0)) == NULL ) { /* * no syntax for this type... return Octet String * syntax. we accomplish this by looking up a well known * attribute type that has that syntax. */ - asi = attr_syntax_get_by_name(ATTR_WITH_OCTETSTRING_SYNTAX); + asi = attr_syntax_get_by_name(ATTR_WITH_OCTETSTRING_SYNTAX, 0); if (asi == NULL) asi = default_asi; } @@ -803,6 +872,20 @@ attr_syntax_insert(struct asyntaxinfo *asip ) }
static void +attr_syntax_insert_tmp(struct asyntaxinfo *asip ) +{ + /* Insert at top of list */ + asip->asi_prev = NULL; + asip->asi_next = global_at_tmp; + if(global_at_tmp){ + global_at_tmp->asi_prev = asip; + global_at_tmp = asip; + } else { + global_at_tmp = asip; + } +} + +static void attr_syntax_remove(struct asyntaxinfo *asip ) { struct asyntaxinfo *prev, *next; @@ -828,7 +911,7 @@ attr_syntax_remove(struct asyntaxinfo *asip ) * Returns an LDAP error code (LDAP_SUCCESS if all goes well). */ int -attr_syntax_add( struct asyntaxinfo *asip ) +attr_syntax_add( struct asyntaxinfo *asip, PRUint32 schema_flags ) { int i, rc = LDAP_SUCCESS; int nolock = asip->asi_flags & SLAPI_ATTR_FLAG_NOLOCKING; @@ -840,7 +923,7 @@ attr_syntax_add( struct asyntaxinfo *asip )
/* make sure the oid is unique */ if ( NULL != ( oldas_from_oid = attr_syntax_get_by_oid_locking_optional( - asip->asi_oid, !nolock))) { + asip->asi_oid, !nolock, schema_flags))) { if ( 0 == (asip->asi_flags & SLAPI_ATTR_FLAG_OVERRIDE)) { /* failure - OID is in use; no override flag */ rc = LDAP_TYPE_OR_VALUE_EXISTS; @@ -852,7 +935,7 @@ attr_syntax_add( struct asyntaxinfo *asip ) * the primary name and OID point to the same schema definition. */ if ( NULL != ( oldas_from_name = attr_syntax_get_by_name_locking_optional( - asip->asi_name, !nolock))) { + asip->asi_name, !nolock, schema_flags))) { if ( 0 == (asip->asi_flags & SLAPI_ATTR_FLAG_OVERRIDE) || ( oldas_from_oid != oldas_from_name )) { /* failure; no override flag OR OID and name don't match */ @@ -860,7 +943,7 @@ attr_syntax_add( struct asyntaxinfo *asip ) goto cleanup_and_return; } /* Flag for deletion. We are going to override this attr */ - attr_syntax_delete(oldas_from_name); + attr_syntax_delete(oldas_from_name, schema_flags); } else if ( NULL != oldas_from_oid ) { /* failure - OID is in use but name does not exist */ rc = LDAP_TYPE_OR_VALUE_EXISTS; @@ -874,11 +957,11 @@ attr_syntax_add( struct asyntaxinfo *asip )
if ( NULL != ( tmpasi = attr_syntax_get_by_name_locking_optional( - asip->asi_aliases[i], !nolock))) { + asip->asi_aliases[i], !nolock, schema_flags))) { if (asip->asi_flags & SLAPI_ATTR_FLAG_OVERRIDE) { /* Flag for tmpasi for deletion. It will be free'd * when attr_syntax_return is called. */ - attr_syntax_delete(tmpasi); + attr_syntax_delete(tmpasi, schema_flags); } else { /* failure - one of the aliases is already in use */ rc = LDAP_TYPE_OR_VALUE_EXISTS; @@ -897,8 +980,8 @@ attr_syntax_add( struct asyntaxinfo *asip ) /* ditto for the override one */ asip->asi_flags &= ~SLAPI_ATTR_FLAG_OVERRIDE; - attr_syntax_add_by_oid( asip->asi_oid, asip, !nolock); - attr_syntax_add_by_name( asip, !nolock); + attr_syntax_add_by_oid( asip->asi_oid, asip, schema_flags, !nolock); + attr_syntax_add_by_name( asip, schema_flags, !nolock);
cleanup_and_return: attr_syntax_return_locking_optional( oldas_from_oid, !nolock ); @@ -1071,7 +1154,7 @@ slapi_attr_type2plugin( const char *type, void **pi ) int slapi_attr_get_oid( const Slapi_Attr *a, char **oid ) { - struct asyntaxinfo *asi = attr_syntax_get_by_name(a->a_type); + struct asyntaxinfo *asi = attr_syntax_get_by_name(a->a_type, 0); if (asi) { *oid = asi->asi_oid; attr_syntax_return(asi); @@ -1087,7 +1170,7 @@ slapi_attr_get_oid( const Slapi_Attr *a, char **oid ) int slapi_attr_get_oid_copy( const Slapi_Attr *a, char **oidp ) { - struct asyntaxinfo *asi = attr_syntax_get_by_name(a->a_type); + struct asyntaxinfo *asi = attr_syntax_get_by_name(a->a_type, 0); if (asi) { *oidp = slapi_ch_strdup( asi->asi_oid ); attr_syntax_return(asi); @@ -1144,7 +1227,7 @@ slapi_attr_is_dn_syntax_type(char *type) int dn_syntax = 0; /* not DN, by default */ struct asyntaxinfo * asi;
- asi = attr_syntax_get_by_name(type); + asi = attr_syntax_get_by_name(type, 0);
if (asi && asi->asi_plugin) { /* If not set, there is no way to get the info */ if ((syntaxoid = asi->asi_plugin->plg_syntax_oid)) { @@ -1323,7 +1406,7 @@ attr_syntax_delete_if_not_flagged(struct asyntaxinfo *asip, void *arg) PR_ASSERT( fi != NULL );
if ( 0 == ( asip->asi_flags & fi->asef_flag )) { - attr_syntax_delete_no_lock( asip, PR_FALSE ); + attr_syntax_delete_no_lock( asip, PR_FALSE, 0 ); return ATTR_SYNTAX_ENUM_REMOVE; } else { return ATTR_SYNTAX_ENUM_NEXT; @@ -1339,7 +1422,7 @@ attr_syntax_force_to_delete(struct asyntaxinfo *asip, void *arg) fi = (struct attr_syntax_enum_flaginfo *)arg; PR_ASSERT( fi != NULL );
- attr_syntax_delete_no_lock( asip, PR_FALSE ); + attr_syntax_delete_no_lock( asip, PR_FALSE, 0 ); return ATTR_SYNTAX_ENUM_REMOVE; }
@@ -1404,6 +1487,7 @@ attr_syntax_delete_all_for_schemareload(unsigned long flag)
#define ATTR_DEFAULT_SYNTAX_OID "1.1" #define ATTR_DEFAULT_SYNTAX "defaultdirstringsyntax" + static int attr_syntax_init(void) { @@ -1413,8 +1497,8 @@ attr_syntax_init(void) if (!oid2asi) { oid2asi = PL_NewHashTable(2047, hashNocaseString, - hashNocaseCompare, - PL_CompareValues, 0, 0); + hashNocaseCompare, + PL_CompareValues, 0, 0); if ( asi_locking && NULL == ( oid2asi_lock = slapi_new_rwlock())) { if(oid2asi) PL_HashTableDestroy(oid2asi); oid2asi = NULL; @@ -1425,11 +1509,19 @@ attr_syntax_init(void) } }
+ if (!oid2asi_tmp) + { + /* temporary hash table for schema reload */ + oid2asi_tmp = PL_NewHashTable(2047, hashNocaseString, + hashNocaseCompare, + PL_CompareValues, 0, 0); + } + if (!name2asi) { name2asi = PL_NewHashTable(2047, hashNocaseString, - hashNocaseCompare, - PL_CompareValues, 0, 0); + hashNocaseCompare, + PL_CompareValues, 0, 0); if ( asi_locking && NULL == ( name2asi_lock = slapi_new_rwlock())) { if(name2asi) PL_HashTableDestroy(name2asi); name2asi = NULL; @@ -1441,10 +1533,18 @@ attr_syntax_init(void) /* add a default syntax plugin as fallback, required during startup */ attr_syntax_create_default( ATTR_DEFAULT_SYNTAX, - ATTR_DEFAULT_SYNTAX_OID, - DIRSTRING_SYNTAX_OID, - SLAPI_ATTR_FLAG_NOUSERMOD| SLAPI_ATTR_FLAG_NOEXPOSE); + ATTR_DEFAULT_SYNTAX_OID, + DIRSTRING_SYNTAX_OID, + SLAPI_ATTR_FLAG_NOUSERMOD| SLAPI_ATTR_FLAG_NOEXPOSE); } + if (!name2asi_tmp) + { + /* temporary hash table for schema reload */ + name2asi_tmp = PL_NewHashTable(2047, hashNocaseString, + hashNocaseCompare, + PL_CompareValues, 0, 0); + } + return 0; }
@@ -1510,7 +1610,7 @@ slapi_add_internal_attr_syntax( const char *name, const char *oid, &asip );
if ( rc == LDAP_SUCCESS ) { - rc = attr_syntax_add( asip ); + rc = attr_syntax_add( asip, 0 ); if ( rc == LDAP_SUCCESS ) { if (attr_syntax_internal_asi_add_ht(asip)) { slapi_log_error(SLAPI_LOG_FATAL, @@ -1518,6 +1618,8 @@ slapi_add_internal_attr_syntax( const char *name, const char *oid, "Failed to stash internal asyntaxinfo: %s.\n", asip->asi_name); } + } else { + attr_syntax_free(asip); } }
@@ -1537,7 +1639,7 @@ attr_syntax_internal_asi_add(struct asyntaxinfo *asip, void *arg) /* Copy is needed since when reloading the schema, * existing syntax info is cleaned up. */ asip_copy = attr_syntax_dup(asip); - rc = attr_syntax_add(asip_copy); + rc = attr_syntax_add(asip_copy, 0); if (LDAP_SUCCESS != rc) { attr_syntax_free(asip_copy); } @@ -1575,3 +1677,37 @@ attr_syntax_find(struct asyntaxinfo *at1, struct asyntaxinfo *at2)
return NULL; } + +/* + * schema reload - now that we have loaded the schema into temporary + * hash tables and linked lists, swap out the old for the new. Then + * reload the internal attr syntaxes + */ +void +attr_syntax_swap_ht() +{ + struct asyntaxinfo *next; + + /* Remove the old hash tables */ + PL_HashTableDestroy(name2asi); + PL_HashTableDestroy(oid2asi); + + /* Free the global attr linked list */ + while(global_at){ + next = global_at->asi_next; + attr_syntax_free(global_at); + global_at = next; + } + + /* + * Swap the hash table/linked list pointers, and set the + * temporary pointers to NULL + */ + name2asi = name2asi_tmp; + name2asi_tmp = NULL; + oid2asi = oid2asi_tmp; + oid2asi_tmp = NULL; + global_at = global_at_tmp; + global_at_tmp = NULL; +} + diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index ebd4fdf..c8a5f8c 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -86,7 +86,7 @@ int op_shared_is_allowed_attr (const char *attr_name, int replicated_op) * check to see if attribute is marked as one clients can't modify */
- asi = attr_syntax_get_by_name( attr_name ); + asi = attr_syntax_get_by_name( attr_name, 0 ); if ( NULL != asi && 0 != ( asi->asi_flags & SLAPI_ATTR_FLAG_NOUSERMOD )) { /* this attribute is not allowed */ diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index eb926c5..48188ed 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -118,7 +118,7 @@ void attr_syntax_write_lock(void); void attr_syntax_unlock_read(void); void attr_syntax_unlock_write(void); int attr_syntax_exists (const char *attr_name); -void attr_syntax_delete ( struct asyntaxinfo *asip ); +void attr_syntax_delete ( struct asyntaxinfo *asip, PRUint32 schema_flags ); #define SLAPI_SYNTAXLENGTH_NONE (-1) /* for syntaxlength parameter */ int attr_syntax_create( const char *attr_oid, char *const*attr_names, const char *attr_desc, const char *attr_superior, @@ -126,18 +126,19 @@ int attr_syntax_create( const char *attr_oid, char *const*attr_names, const char *mr_substring, schemaext *extensions, const char *attr_syntax, int syntaxlength, unsigned long flags, struct asyntaxinfo **asip ); void attr_syntax_free( struct asyntaxinfo *a ); -int attr_syntax_add( struct asyntaxinfo *asip ); +int attr_syntax_add( struct asyntaxinfo *asip, PRUint32 schema_flags ); char *attr_syntax_normalize_no_lookup( const char *s ); char *attr_syntax_normalize_no_lookup_ext( char *s, int flags ); void attr_syntax_enumerate_attrs(AttrEnumFunc aef, void *arg, PRBool writelock); void attr_syntax_all_clear_flag( unsigned long flag ); void attr_syntax_delete_all_not_flagged( unsigned long flag ); -struct asyntaxinfo *attr_syntax_get_by_oid ( const char *oid ); -struct asyntaxinfo *attr_syntax_get_by_name ( const char *name ); +struct asyntaxinfo *attr_syntax_get_by_oid ( const char *oid, PRUint32 schema_flags ); +struct asyntaxinfo *attr_syntax_get_by_name ( const char *name, PRUint32 schema_flags ); struct asyntaxinfo *attr_syntax_get_by_name_with_default ( const char *name ); -struct asyntaxinfo *attr_syntax_get_by_name_locking_optional ( const char *name, PRBool use_lock ); +struct asyntaxinfo *attr_syntax_get_by_name_locking_optional ( const char *name, PRBool use_lock, PRUint32 schema_flags ); struct asyntaxinfo *attr_syntax_get_global_at(); struct asyntaxinfo *attr_syntax_find(struct asyntaxinfo *at1, struct asyntaxinfo *at2); +void attr_syntax_swap_ht(void); /* * Call attr_syntax_return() when you are done using a value returned * by attr_syntax_get_by_oid() or attr_syntax_get_by_name(). diff --git a/ldap/servers/slapd/schema.c b/ldap/servers/slapd/schema.c index 8744a6d..52f29c2 100644 --- a/ldap/servers/slapd/schema.c +++ b/ldap/servers/slapd/schema.c @@ -2589,7 +2589,7 @@ schema_delete_attributes ( Slapi_Entry *entryBefore, LDAPMod *mod, sscanf (attr_ldif, "%s name %s syntax %s", psbAttrOid->buffer, psbAttrName->buffer, psbAttrSyntax->buffer); - if ((a = attr_syntax_get_by_name ( psbAttrName->buffer)) != NULL ) { + if ((a = attr_syntax_get_by_name ( psbAttrName->buffer, 0 )) != NULL ) { /* only modify attrs which were user defined */ if (a->asi_flags & SLAPI_ATTR_FLAG_STD_ATTR) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, @@ -2641,7 +2641,7 @@ schema_delete_attributes ( Slapi_Entry *entryBefore, LDAPMod *mod, }
/* Delete it. */ - attr_syntax_delete( a ); + attr_syntax_delete( a, 0 ); attr_syntax_return( a ); } else { @@ -2764,7 +2764,7 @@ add_oc_internal(struct objclass *pnew_oc, char *errorbuf, size_t errorbufsize, }
/* check to see if the oid is already in use by an attribute */ - if (!rc && (pasyntaxinfo = attr_syntax_get_by_oid(pnew_oc->oc_oid))) { + if (!rc && (pasyntaxinfo = attr_syntax_get_by_oid(pnew_oc->oc_oid, flags))) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_oc, pnew_oc->oc_name, "The OID "%s" is also used by the attribute type "%s"", @@ -2883,7 +2883,7 @@ schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, * handle the various cases. */ if ( NULL == ( oldasip = - attr_syntax_get_by_oid( newasip->asi_oid ))) { + attr_syntax_get_by_oid( newasip->asi_oid, 0 ))) { /* new attribute type */ LDAPDebug( LDAP_DEBUG_TRACE, "schema_replace_attributes:" " new type %s (OID %s)\n", @@ -2901,14 +2901,14 @@ schema_replace_attributes ( Slapi_PBlock *pb, LDAPMod *mod, char *errorbuf, " replacing type %s (OID %s)\n", newasip->asi_name, newasip->asi_oid, 0 ); /* flag for deletion */ - attr_syntax_delete( oldasip ); + attr_syntax_delete( oldasip, 0 ); }
attr_syntax_return( oldasip ); }
if ( NULL != newasip ) { /* add new or replacement definition */ - rc = attr_syntax_add( newasip ); + rc = attr_syntax_add( newasip, 0 ); if ( LDAP_SUCCESS != rc ) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, newasip->asi_name, @@ -3862,7 +3862,7 @@ read_at_ldif(const char *input, struct asyntaxinfo **asipp, char *errorbuf, if (!status && (NULL != pSuperior)) { struct asyntaxinfo *asi_parent;
- asi_parent = attr_syntax_get_by_name(pSuperior); + asi_parent = attr_syntax_get_by_name(pSuperior, schema_flags); /* if we find no match then server won't start or add the attribute type */ if (asi_parent == NULL) { LDAPDebug (LDAP_DEBUG_PARSE, @@ -3963,7 +3963,7 @@ read_at_ldif(const char *input, struct asyntaxinfo **asipp, char *errorbuf, struct asyntaxinfo *tmpasi;
if (!(flags & SLAPI_ATTR_FLAG_OVERRIDE) && - ( NULL != ( tmpasi = attr_syntax_get_by_oid(pOid)))) { + ( NULL != ( tmpasi = attr_syntax_get_by_oid(pOid, schema_flags)))) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, first_attr_name, "Could not be added because the OID "%s" is already in use", @@ -3985,7 +3985,7 @@ read_at_ldif(const char *input, struct asyntaxinfo **asipp, char *errorbuf, if ( NULL != asipp ) { *asipp = tmpasip; /* just return it */ } else { /* add the new attribute to the global store */ - status = attr_syntax_add( tmpasip ); + status = attr_syntax_add( tmpasip, schema_flags ); if ( LDAP_SUCCESS != status ) { if ( 0 != (flags & SLAPI_ATTR_FLAG_OVERRIDE) && LDAP_TYPE_OR_VALUE_EXISTS == status ) { @@ -4173,10 +4173,10 @@ parse_attr_str(const char *input, struct asyntaxinfo **asipp, char *errorbuf, if (NULL != atype->at_sup_oid) { struct asyntaxinfo *asi_parent;
- asi_parent = attr_syntax_get_by_name(atype->at_sup_oid); + asi_parent = attr_syntax_get_by_name(atype->at_sup_oid, schema_flags); /* if we find no match then server won't start or add the attribute type */ if (asi_parent == NULL) { - LDAPDebug (LDAP_DEBUG_PARSE, "Cannot find parent attribute type "%s"\n", + LDAPDebug (LDAP_DEBUG_ANY, "Cannot find parent attribute type "%s"\n", atype->at_sup_oid, NULL, NULL); schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, first_attr_name, "Missing parent attribute syntax OID"); @@ -4342,7 +4342,7 @@ parse_attr_str(const char *input, struct asyntaxinfo **asipp, char *errorbuf, /* * Check if the OID is already being used */ - if (!(flags & SLAPI_ATTR_FLAG_OVERRIDE) && ( NULL != (tmpasi = attr_syntax_get_by_oid(atype->at_oid)))) { + if (!(flags & SLAPI_ATTR_FLAG_OVERRIDE) && ( NULL != (tmpasi = attr_syntax_get_by_oid(atype->at_oid, schema_flags)))) { schema_create_errormsg( errorbuf, errorbufsize, schema_errprefix_at, first_attr_name, "Could not be added because the OID "%s" is already in use", atype->at_oid); @@ -4361,7 +4361,7 @@ parse_attr_str(const char *input, struct asyntaxinfo **asipp, char *errorbuf, *asipp = tmpasip; /* just return it */ } else { /* add the new attribute to the global store */ - status = attr_syntax_add( tmpasip ); + status = attr_syntax_add( tmpasip, schema_flags ); if ( LDAP_SUCCESS != status ) { if ( 0 != (flags & SLAPI_ATTR_FLAG_OVERRIDE) && LDAP_TYPE_OR_VALUE_EXISTS == status ) { @@ -6054,9 +6054,13 @@ slapi_validate_schema_files(char *schemadir) }
/* - * API to reload the schema files. - * Rule: this function is called when slapi_validate_schema_files is passed. - * Schema checking is skipped in this function. + * Reload the schema files + * + * This is only called from the schema_reload task. The flag DSE_SCHEMA_LOCKED + * is also only set been called from this function. To not interrupt clients + * we will rebuild the schema in separate hash tables, and then swap the + * hash tables once the schema is completely reloaded. We use the DSE_SCHEMA_LOCKED + * flag to tell the attribute syntax functions to use the temporary hashtables. */ int slapi_reload_schema_files(char *schemadir) @@ -6074,14 +6078,21 @@ slapi_reload_schema_files(char *schemadir) } slapi_be_Wlock(be); /* be lock must be outer of schemafile lock */ reload_schemafile_lock(); - /* Exclude attr_syntax not to grab from the hash table while cleaning up */ - attr_syntax_write_lock(); - attr_syntax_delete_all_for_schemareload(SLAPI_ATTR_FLAG_KEEP); oc_delete_all_nolock(); - attr_syntax_unlock_write(); rc = init_schema_dse_ext(schemadir, be, &my_pschemadse, DSE_SCHEMA_NO_CHECK | DSE_SCHEMA_LOCKED); if (rc) { + /* + * The schema has been reloaded into the temporary hash tables. + * Take the write lock, wipe out the existing hash tables, and + * swap in the new ones. + */ + attr_syntax_write_lock(); + attr_syntax_delete_all_for_schemareload(SLAPI_ATTR_FLAG_KEEP); + attr_syntax_swap_ht(); + attr_syntax_unlock_write(); + slapi_reload_internal_attr_syntax(); + dse_destroy(pschemadse); pschemadse = my_pschemadse; reload_schemafile_unlock();
389-commits@lists.fedoraproject.org
-
Mark Reynolds