This is an automated email from the git hooks/post-receive script.
tbordaz pushed a commit to branch 389-ds-base-1.4.1
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.1 by this push:
new d3c792b Ticket 51068 - deadlock when updating the schema
d3c792b is described below
commit d3c792baf0e8a05160c24d067c5832f6f7c65a20
Author: Thierry Bordaz <tbordaz(a)redhat.com>
AuthorDate: Tue May 5 17:44:01 2020 +0200
Ticket 51068 - deadlock when updating the schema
Bug Description:
It exists a 3 threads deadlock scenario. It involves state change plugins when
it
calls schema_changed_callback. So the trigger is a change of schema (direct or
via
replication). The scenario is
MOD(cn=schema) hold StateChange lock wait for vattr lock
SRCH hold vattr lock wait for DB page
MOD hold DB page wait for StateChange lock
Fix Description:
Statechange lock protects the list of registered callbacks.
lock is a mutex where actually registration of callback is only done
at startup. Later the list is only lookup.
Making statechange lock a rwlock suppresses the deadlock scenario
as MODs will only acquire in read StateChange lock.
It should also improve performance as at the moment all MODs are serialized
on that lock
In order to prevent writer starvation a new slapi_new_rwlock_prio
create rwlock with priority to writers.
https://pagure.io/389-ds-base/issue/51068
Reviewed by: Mark Reynolds, William Brown
Platforms tested: 30
Flag Day: no
Doc impact: no
---
ldap/servers/plugins/statechange/statechange.c | 24 +++++++++++++-----------
ldap/servers/slapd/slapi-plugin.h | 16 ++++++++++++++++
ldap/servers/slapd/slapi2nspr.c | 25 +++++++++++++++++++++++++
ldap/servers/slapd/vattr.c | 2 +-
4 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/ldap/servers/plugins/statechange/statechange.c
b/ldap/servers/plugins/statechange/statechange.c
index c6a611d..9026382 100644
--- a/ldap/servers/plugins/statechange/statechange.c
+++ b/ldap/servers/plugins/statechange/statechange.c
@@ -40,7 +40,7 @@ static SCNotify *head; /* a place to start in the list */
#define SCN_PLUGIN_SUBSYSTEM "statechange-plugin" /* used for logging */
static void *api[5];
-static Slapi_Mutex *buffer_lock = 0;
+static Slapi_RWLock *buffer_lock = 0;
static PRUint64 g_plugin_started = 0;
/*
@@ -139,7 +139,7 @@ statechange_start(Slapi_PBlock *pb __attribute__((unused)))
api[3] = (void *)_statechange_unregister_all;
api[4] = (void *)_statechange_vattr_cache_invalidator_callback;
- if (0 == (buffer_lock = slapi_new_mutex())) /* we never free this mutex */
+ if (0 == (buffer_lock = slapi_new_rwlock()))
{
/* badness */
slapi_log_err(SLAPI_LOG_ERR, SCN_PLUGIN_SUBSYSTEM, "statechange_start -
Failed to create lock\n");
@@ -179,7 +179,9 @@ statechange_close(Slapi_PBlock *pb __attribute__((unused)))
slapi_counter_destroy(&op_counter);
slapi_apib_unregister(StateChange_v1_0_GUID);
- slapi_destroy_mutex(buffer_lock);
+ if (buffer_lock) {
+ slapi_destroy_rwlock(buffer_lock);
+ }
buffer_lock = NULL;
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<--
statechange_close\n");
@@ -239,7 +241,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "-->
statechange_post_op\n");
/* evaluate this operation against the notification entries */
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_rdlock(buffer_lock);
if (head) {
slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
if (NULL == sdn) {
@@ -289,7 +291,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
} while (notify && notify != head);
}
bail:
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<--
statechange_post_op\n");
return SLAPI_PLUGIN_SUCCESS; /* always succeed */
@@ -337,7 +339,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void
*caller_data
}
item->func = func;
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (head == NULL) {
head = item;
head->next = head;
@@ -348,7 +350,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void
*caller_data
head->prev = item;
item->prev->next = item;
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_ch_free_string(&writable_filter);
ret = SLAPI_PLUGIN_SUCCESS;
@@ -370,7 +372,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback
thefunc)
return ret;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if ((func = statechange_find_notify(dn, filter, thefunc))) {
func->prev->next = func->next;
@@ -391,7 +393,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback
thefunc)
slapi_ch_free((void **)&func);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
return ret;
@@ -409,7 +411,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback
callback)
return;
}
- slapi_lock_mutex(buffer_lock);
+ slapi_rwlock_wrlock(buffer_lock);
if (notify) {
do {
@@ -440,7 +442,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback
callback)
} while (notify != start_notify && notify != NULL);
}
- slapi_unlock_mutex(buffer_lock);
+ slapi_rwlock_unlock(buffer_lock);
slapi_counter_decrement(op_counter);
}
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 596824c..ff32f87 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6087,6 +6087,22 @@ int slapi_wait_condvar(Slapi_CondVar *cvar, struct timeval
*timeout);
int slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all);
/**
+ * Creates a new read/write lock
+ * If prio_writer the rwlock gives priority on writers
+ * else it give priority on readers (default)
+ *
+ * \return A pointer to a \c Slapi_RWLock
+ *
+ * \note Free the returned lock by calling slapi_destroy_rwlock() when finished
+ *
+ * \see slapi_destroy_rwlock()
+ * \see slapi_rwlock_rdlock()
+ * \see slapi_rwlock_wrlock()
+ * \see slapi_rwlock_unlock()
+ */
+Slapi_RWLock *slapi_new_rwlock_prio(int32_t prio_writer);
+
+/**
* Creates a new read/write lock.
*
* \return A pointer to a \c Slapi_RWLock
diff --git a/ldap/servers/slapd/slapi2nspr.c b/ldap/servers/slapd/slapi2nspr.c
index b3e6d94..232d159 100644
--- a/ldap/servers/slapd/slapi2nspr.c
+++ b/ldap/servers/slapd/slapi2nspr.c
@@ -182,6 +182,31 @@ slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all)
}
Slapi_RWLock *
+slapi_new_rwlock_prio(int32_t prio_writer)
+{
+#ifdef USE_POSIX_RWLOCKS
+ pthread_rwlock_t *rwlock = NULL;
+ pthread_rwlockattr_t attr;
+
+ pthread_rwlockattr_init(&attr);
+ if (prio_writer) {
+ pthread_rwlockattr_setkind_np(&attr,
PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+ } else {
+ pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+ }
+
+ rwlock = (pthread_rwlock_t *)slapi_ch_malloc(sizeof(pthread_rwlock_t));
+ if (rwlock) {
+ pthread_rwlock_init(rwlock, &attr);
+ }
+
+ return ((Slapi_RWLock *)rwlock);
+#else
+ return ((Slapi_RWLock *)PR_NewRWLock(PR_RWLOCK_RANK_NONE,
"slapi_rwlock"));
+#endif
+}
+
+Slapi_RWLock *
slapi_new_rwlock(void)
{
#ifdef USE_POSIX_RWLOCKS
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index d8b2c83..7c70f41 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -1996,7 +1996,7 @@ vattr_map_create(void)
return ENOMEM;
}
- the_map->lock = slapi_new_rwlock();
+ the_map->lock = slapi_new_rwlock_prio(1 /* priority on writers */);
if (NULL == the_map) {
slapd_nasty(sourcefile, 3, 0);
return ENOMEM;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.