PR#19: Bug 1435663: deadlock between write and search operation in schema-compat plugin - slapi-nis

slapi-nis

#19 Bug 1435663: deadlock between write and search operation in schema-compat plugin

Merged 5 years ago by abbra. Opened 5 years ago by tbordaz.

tbordaz/slapi-nis bz_1435663 into master

Bug 1435663: deadlock between write and search operation in schema-compat plugin

Thierry Bordaz • 5 years ago

8b10ea4

src/back-sch.c

file modified

+221

		`@@ -2304,6 +2304,157 @@`
		`return state->use_be_txns ? backend_write_cb(pb, state) : 0;`
		`}`

		`+ static int`
		`+ backend_be_pre_write_cb(Slapi_PBlock *pb) {`
		`+ int ret;`
		`+ int lock_status;`
		`+ int lock_count;`
		`+ struct plugin_state *state;`
		`+`
		`+ slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &state);`
		`+`
		`+ if (wrap_get_call_level() > 0) {`
		`+ return 0;`
		`+ }`
		`+ if (state->ready_to_serve == 0) {`
		`+ /* No data to serve yet */`
		`+ return 0;`
		`+ }`
		`+ if (rw_monitor_enabled() == MAP_MONITOR_DISABLED) {`
		`+ return 0;`
		`+ }`
		`+`
		`+`
		`+ wrap_inc_call_level();`
		`+`
		`+ lock_status = get_plugin_monitor_status();`
		`+ lock_count = get_plugin_monitor_count();`
		`+ if (lock_status == MAP_RWLOCK_UNINIT) {`
		`+ return 0;`
		`+ }`
		`+`
		`+ if (lock_status == MAP_RWLOCK_FREE) {`
		`+ set_plugin_monitor_count(1);`
		`+`
		`+ if (plugin_wrlock() == 0) {`
		`+ ret = 0;`
		`+ } else {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "backend_be_pre_write_cb: unable to acquire write lock\n");`
		`+ ret = -1;`
		`+ }`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "backend_be_pre_write_cb: (%p) MAP_RWLOCK_FREE -> MAP_WLOCK_HELD : count=%d\n", PR_GetCurrentThread(), 1);`
		`+ #endif`
		`+ } else {`
		`+ set_plugin_monitor_count(lock_count + 1);`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "backend_be_pre_write_cb: (%p) %s --> MAP_WLOCK_HELD : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD": "MAP_RLOCK_HELD",`
		`+ lock_count + 1);`
		`+ #endif`
		`+ ret = 0;`
		`+ if (lock_status == MAP_RLOCK_HELD) {`
		`+ /* lock is already acquired in read */`
		`+`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map backend_be_pre_write_cb: weird situation map lock is held in read and now required in write mode\n");`
		`+ #endif`
		`+ /* First free the lock held in read */`
		`+ ret = plugin_unlock();`
		`+ if (ret) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "backend_be_pre_write_cb: fail to unlock plugin lock (%d)\n", ret);`
		`+ }`
		`+`
		`+ /* Second acquire it in write */`
		`+ ret = plugin_wrlock();`
		`+ if (ret) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "backend_be_pre_write_cb: fail to write lock plugin lock (%d)\n", ret);`
		`+ }`
		`+ }`
		`+ }`
		`+`
		`+ set_plugin_monitor_status(MAP_WLOCK_HELD);`
		`+ wrap_dec_call_level();`
		`+`
		`+ return ret;`
		`+ }`
		`+`
		`+ static int`
		`+ backend_be_post_write_cb(Slapi_PBlock *pb)`
		`+ {`
		`+ int ret;`
		`+ int lock_status;`
		`+ int lock_count;`
		`+ struct plugin_state *state;`
		`+`
		`+ slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &state);`
		`+`
		`+ if (wrap_get_call_level() > 0) {`
		`+ return 0;`
		`+ }`
		`+ if (state->ready_to_serve == 0) {`
		`+ /* No data to serve yet */`
		`+ return 0;`
		`+ }`
		`+ if (rw_monitor_enabled() == MAP_MONITOR_DISABLED) {`
		`+ return 0;`
		`+ }`
		`+`
		`+ wrap_inc_call_level();`
		`+`
		`+ lock_status = get_plugin_monitor_status();`
		`+ lock_count = get_plugin_monitor_count();`
		`+ if (lock_status == MAP_RWLOCK_UNINIT) {`
		`+ return 0;`
		`+ }`
		`+`
		`+ if (lock_count == 1) {`
		`+ set_plugin_monitor_status(MAP_RWLOCK_FREE);`
		`+ if (plugin_unlock() == 0) {`
		`+ ret = 0;`
		`+ } else {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "backend_be_post_write_cb: unable to release write lock\n");`
		`+ ret = -1;`
		`+ }`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "backend_be_post_write_cb: (%p) %s --> MAP_RWLOCK_FREE : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD" : (lock_status == MAP_RLOCK_HELD) ? "MAP_RLOCK_HELD" : "MAP_RWLOCK_FREE",`
		`+ 0);`
		`+ #endif`
		`+ }`
		`+ if (lock_count >= 1) {`
		`+ set_plugin_monitor_count(lock_count - 1);`
		`+ #if DEBUG_MAP_LOCK`
		`+ if (lock_count > 1) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "backend_be_post_write_cb: (%p) keep %s : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD" : (lock_status == MAP_RLOCK_HELD) ? "MAP_RLOCK_HELD" : "MAP_RWLOCK_FREE",`
		`+ lock_count - 1);`
		`+ } else {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "backend_be_post_write_cb: (%p) is now %s : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ "MAP_RWLOCK_FREE",`
		`+ lock_count - 1);`
		`+ }`
		`+ #endif`
		`+ }`
		`+ wrap_dec_call_level();`
		`+`
		`+ return ret;`
		`+ }`
		`+`
		`#ifdef USE_PAM`
		`static int`
		`backend_bind_cb_pam(Slapi_PBlock pb, const char username, char *ndn)`
		`@@ -2815,12 +2966,82 @@`
		`}`

		`int`
		`+ backend_init_be_preop(Slapi_PBlock pb, struct plugin_state state)`
		`+ {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "hooking up bet preoperation callbacks\n");`
		`+ /* Intercept write requests and return an insufficient-access error for`
		`+ * attempts to write to anything we're managing. */`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_ADD_FN,`
		`+ backend_be_pre_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn pre add callback\n");`
		`+ return -1;`
		`+ }`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_MODIFY_FN,`
		`+ backend_be_pre_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn pre modify callback\n");`
		`+ return -1;`
		`+ }`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_MODRDN_FN,`
		`+ backend_be_pre_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn pre modrdn callback\n");`
		`+ return -1;`
		`+ }`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_PRE_DELETE_FN,`
		`+ backend_be_pre_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn pre delete callback\n");`
		`+ return -1;`
		`+ }`
		`+ /* We don't hook abandonment requests. */`
		`+ /* We don't hook unbind requests. */`
		`+ return 0;`
		`+ }`
		`+`
		`+ int`
		`backend_init_betxn_postop(Slapi_PBlock pb, struct plugin_state state)`
		`{`
		`slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`"hooking up betxn postoperation callbacks\n");`
		`return backend_shr_betxn_postop_init(pb, state);`
		`}`
		`+`
		`+`
		`+ int`
		`+ backend_init_be_postop(Slapi_PBlock pb, struct plugin_state state)`
		`+ {`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_ADD_FN,`
		`+ backend_be_post_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn post add callback\n");`
		`+ return -1;`
		`+ }`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_MODIFY_FN,`
		`+ backend_be_post_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn post modify "`
		`+ "callback\n");`
		`+ return -1;`
		`+ }`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_MODRDN_FN,`
		`+ backend_be_post_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn post modrdn "`
		`+ "callback\n");`
		`+ return -1;`
		`+ }`
		`+ if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_DELETE_FN,`
		`+ backend_be_post_write_cb) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error hooking up betxn post delete "`
		`+ "callback\n");`
		`+ return -1;`
		`+ }`
		`+ return 0;`
		`+ }`
		`#endif`

		`int`

src/back-shr.c

file modified

+94

		`@@ -2866,3 +2866,97 @@`
		`}`
		`return 0;`
		`}`
		`+`
		`+ static unsigned int thread_dummy;`
		`+ static unsigned int thread_plugin_lock_status;`
		`+ static int thread_plugin_lock_count;`
		`+`
		`+ void`
		`+ init_map_lock(void)`
		`+ {`
		`+ /* The plugin lock is initialized as free */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "init_plugin_lock",`
		`+ "thread_id = %p\n", PR_GetCurrentThread());`
		`+`
		`+ PR_NewThreadPrivateIndex (&thread_dummy, NULL);`
		`+ PR_NewThreadPrivateIndex(&thread_plugin_lock_status, NULL);`
		`+ PR_NewThreadPrivateIndex(&thread_plugin_lock_count, NULL);`
		`+`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "init_plugin_lock",`
		`+ "thread_plugin_lock_status = %d\n", thread_plugin_lock_status);`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "init_map_lock",`
		`+ "thread_plugin_lock_count = %d\n", thread_plugin_lock_count);`
		`+ }`
		`+`
		`+ int`
		`+ rw_monitor_enabled(void)`
		`+ {`
		`+ if (thread_plugin_lock_status)`
		`+ return (int) MAP_MONITOR_ENABLED;`
		`+ else`
		`+ return (int) MAP_MONITOR_DISABLED;`
		`+ }`
		`+`
		`+ int`
		`+ get_plugin_monitor_status(void)`
		`+ {`
		`+ int ret;`
		`+`
		`+ if (thread_plugin_lock_status)`
		`+ ret = (int) PR_GetThreadPrivate(thread_plugin_lock_status);`
		`+ else`
		`+ ret = (int) MAP_RWLOCK_UNINIT;`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "get_plugin_monitor_status",`
		`+ "lock_status = %d (%p)\n", ret, PR_GetCurrentThread());`
		`+ #endif`
		`+ return ret;`
		`+ }`
		`+`
		`+ void`
		`+ set_plugin_monitor_status(int lock_status)`
		`+ {`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "set_plugin_monitor_status",`
		`+ "lock_status = %d --> %d (%p)\n", get_plugin_monitor_status(), (int) lock_status, PR_GetCurrentThread());`
		`+ #endif`
		`+`
		`+ if (thread_plugin_lock_status)`
		`+ PR_SetThreadPrivate(thread_plugin_lock_status, (void *) lock_status);`
		`+ else`
		`+ PR_SetThreadPrivate(thread_plugin_lock_status, (void *) MAP_RWLOCK_UNINIT);`
		`+`
		`+`
		`+ }`
		`+`
		`+ int`
		`+ get_plugin_monitor_count(void)`
		`+ {`
		`+ int ret;`
		`+`
		`+ if (thread_plugin_lock_count)`
		`+ ret = (int) PR_GetThreadPrivate(thread_plugin_lock_count);`
		`+ else`
		`+ ret = (int) MAP_RWLOCK_UNINIT;`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "get_plugin_monitor_count",`
		`+ "lock_count = %d (%p)\n", ret, PR_GetCurrentThread());`
		`+ #endif`
		`+ return ret;`
		`+ }`
		`+`
		`+ void`
		`+ set_plugin_monitor_count(int lock_count)`
		`+ {`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "set_plugin_monitor_count",`
		`+ "lock_count = %d --> %d (%p)\n", get_plugin_monitor_count(), (int) lock_count, PR_GetCurrentThread());`
		`+ #endif`
		`+`
		`+ if (thread_plugin_lock_count)`
		`+ PR_SetThreadPrivate(thread_plugin_lock_count, (void *) lock_count);`
		`+ else`
		`+ PR_SetThreadPrivate(thread_plugin_lock_count, (void *) MAP_RWLOCK_UNINIT);`
		`+`
		`+`
		`+ }`

src/back-shr.h

file modified

+16

		`@@ -22,6 +22,22 @@`
		`#ifndef back_shr_h`
		`#define back_shr_h`

		`+ #define DEBUG_MAP_LOCK 0`
		`+ #define MAP_MONITOR_DISABLED 0`
		`+ #define MAP_MONITOR_ENABLED 1`
		`+`
		`+ #define MAP_RWLOCK_UNINIT 3`
		`+ #define MAP_WLOCK_HELD 2`
		`+ #define MAP_RLOCK_HELD 1`
		`+ #define MAP_RWLOCK_FREE 0`
		`+ int rw_monitor_enabled(void);`
		`+ int get_plugin_monitor_status(void);`
		`+ void set_plugin_monitor_status(int lock_status);`
		`+ int get_plugin_monitor_count(void);`
		`+ void set_plugin_monitor_count(int lock_count);`
		`+ void init_map_lock(void);`
		`+`
		`+`
		`struct plugin_state;`

		`void backend_shr_free_server_name(struct plugin_state state, char master);`

src/map.c

file modified

+265 -6

		`@@ -45,6 +45,7 @@`
		`#include "map.h"`
		`#include "portmap.h"`
		`#include "wrap.h"`
		`+ #include "back-shr.h"`

		`/* The singleton for the cache. */`
		`static struct {`
		`@@ -91,6 +92,7 @@`
		`} *domains;`
		`int n_domains;`
		`struct wrapped_rwlock *lock;`
		`+ struct wrapped_rwlock *plugin_lock;`
		`} map_data;`

		`static void *`
		`@@ -1155,6 +1157,10 @@`
		`if (map_data.lock == NULL) {`
		`return -1;`
		`}`
		`+ map_data.plugin_lock = wrap_new_rwlock();`
		`+ if (map_data.plugin_lock == NULL) {`
		`+ return -1;`
		`+ }`
		`return 0;`
		`}`

		`@@ -1193,6 +1199,8 @@`
		`}`
		`wrap_free_rwlock(map_data.lock);`
		`map_data.lock = NULL;`
		`+ wrap_free_rwlock(map_data.plugin_lock);`
		`+ map_data.plugin_lock = NULL;`
		`}`

		`int`
		`@@ -1219,19 +1227,270 @@`
		`}`

		`int`
		`- map_rdlock(void)`
		`+ plugin_rdlock(void)`
		`{`
		`- return wrap_rwlock_rdlock(map_data.lock);`
		`+ return wrap_rwlock_rdlock(map_data.plugin_lock);`
		`}`

		`int`
		`- map_wrlock(void)`
		`+ plugin_wrlock(void)`
		`{`
		`- return wrap_rwlock_wrlock(map_data.lock);`
		`+ return wrap_rwlock_wrlock(map_data.plugin_lock);`
		`}`

		`int`
		`- map_unlock(void)`
		`+ plugin_unlock(void)`
		`{`
		`- return wrap_rwlock_unlock(map_data.lock);`
		`+ return wrap_rwlock_unlock(map_data.plugin_lock);`
		`}`
		`+`
		`+ int`
		`+ map_rdlock(void)`
		`+ {`
		`+ int lock_status;`
		`+ int lock_count;`
		`+ int rc = 0;`
		`+`
		`+ if (rw_monitor_enabled() == MAP_MONITOR_DISABLED) {`
		`+ /* This is not initialized used the old way */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: old way MAP_MONITOR_DISABLED\n");`
		`+ return wrap_rwlock_rdlock(map_data.lock);`
		`+ }`
		`+`
		`+`
		`+ lock_status = get_plugin_monitor_status();`
		`+ lock_count = get_plugin_monitor_count();`
		`+`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "map_rdlock",`
		`+ "thread_id = %p (call level = %d)\n", PR_GetCurrentThread(), wrap_get_call_level());`
		`+ #endif`
		`+ if (lock_status == MAP_RWLOCK_UNINIT) {`
		`+ /* This is not initialized used the old way */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: old way lock_status == MAP_RWLOCK_UNINIT\n");`
		`+ return wrap_rwlock_rdlock(map_data.lock);`
		`+ }`
		`+`
		`+ if (lock_status == MAP_RWLOCK_FREE) {`
		`+ /* The plugin lock is free, acquire it */`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: current lock_status == MAP_RWLOCK_FREE\n");`
		`+ #endif`
		`+ set_plugin_monitor_status(MAP_RLOCK_HELD);`
		`+ set_plugin_monitor_count(1);`
		`+ #if DEBUG_MAP_LOCK`
		`+ if (lock_count != 0) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: (%p) ALERT !!! count was %d -> 1\n", PR_GetCurrentThread(), lock_count);`
		`+ }`
		`+ #endif`
		`+`
		`+ /* Acquire the slapi plugin in read */`
		`+ rc = plugin_rdlock();`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: (%p) MAP_RWLOCK_FREE -> MAP_RLOCK_HELD: fail to read lock plugin lock (%d)\n", PR_GetCurrentThread(), rc);`
		`+ return rc;`
		`+ }`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: (%p) MAP_RWLOCK_FREE -> MAP_RLOCK_HELD : count=%d\n", PR_GetCurrentThread(), 1);`
		`+ #endif`
		`+ rc = wrap_rwlock_rdlock(map_data.lock);`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "Fail to acquire map lock in read (%d)\n", rc);`
		`+ plugin_unlock();`
		`+ return rc;`
		`+ }`
		`+ return 0;`
		`+ }`
		`+`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map rdlock: (%p) was already hold %s : count=%d > %d!!!\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD": "MAP_RLOCK_HELD",`
		`+ lock_count, lock_count + 1);`
		`+ #endif`
		`+ set_plugin_monitor_count(lock_count + 1);`
		`+ return 0;`
		`+ }`
		`+`
		`+ int`
		`+ map_wrlock(void)`
		`+ {`
		`+ int lock_status;`
		`+ int lock_count;`
		`+ int rc = 0;`
		`+`
		`+ if (rw_monitor_enabled() == MAP_MONITOR_DISABLED) {`
		`+ /* This is not initialized used the old way */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: old way MAP_MONITOR_DISABLED\n");`
		`+ return wrap_rwlock_wrlock(map_data.lock);`
		`+ }`
		`+`
		`+ lock_status = get_plugin_monitor_status();`
		`+ lock_count = get_plugin_monitor_count();`
		`+`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "map wrlock",`
		`+ "thread_id = %p (call level = %d)\n", PR_GetCurrentThread(), wrap_get_call_level());`
		`+ #endif`
		`+ if (lock_status == MAP_RWLOCK_UNINIT) {`
		`+ /* This is not initialized used the old way */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: old way lock_status == MAP_LOCK_UNINIT\n");`
		`+`
		`+ return wrap_rwlock_wrlock(map_data.lock);`
		`+ }`
		`+`
		`+ if (lock_status == MAP_RWLOCK_FREE) {`
		`+ /* The lock is free, acquire it */`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: current lock_status == MAP_LOCK_FREE\n");`
		`+ #endif`
		`+`
		`+ set_plugin_monitor_count(1);`
		`+ #if DEBUG_MAP_LOCK`
		`+ if (lock_count != 0) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: (%p) ALERT !!! count was %d --> 1\n", PR_GetCurrentThread(), lock_count);`
		`+ }`
		`+ #endif`
		`+ /* Acquire the slapi plugin in write */`
		`+ rc = plugin_wrlock();`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schemacompat",`
		`+ "map wrlock: (%p) MAP_RWLOCK_FREE -> MAP_RLOCK_HELD: fail to read lock plugin lock (%d)\n", PR_GetCurrentThread(), rc);`
		`+ return rc;`
		`+ }`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: (%p) MAP_RWLOCK_FREE --> MAP_WLOCK_HELD : count=%d\n", PR_GetCurrentThread(), 1);`
		`+ #endif`
		`+`
		`+ rc = wrap_rwlock_wrlock(map_data.lock);`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: (%p) MAP_RWLOCK_FREE --> MAP_WLOCK_HELD : fail to write lock map lock (%d)\n", PR_GetCurrentThread(), rc);`
		`+ plugin_unlock();`
		`+ goto common;`
		`+ }`
		`+ } else {`
		`+ set_plugin_monitor_count(lock_count + 1);`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: (%p) %s --> MAP_WLOCK_HELD : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD": "MAP_RLOCK_HELD",`
		`+ lock_count + 1);`
		`+ #endif`
		`+`
		`+ if (lock_status == MAP_RLOCK_HELD) {`
		`+ /* lock is already acquired in read */`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: weird situation map lock is held in read and now required in write mode\n");`
		`+ #endif`
		`+ /* First free the lock held in read */`
		`+ rc = plugin_unlock();`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: fail to unlock plugin lock (%d)\n", rc);`
		`+ goto common;`
		`+ }`
		`+`
		`+ /* Second acquire it in write */`
		`+ rc = plugin_wrlock();`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map wrlock: fail to write lock plugin lock (%d)\n", rc);`
		`+ goto common;`
		`+ }`
		`+ }`
		`+ }`
		`+`
		`+ common:`
		`+ set_plugin_monitor_status(MAP_WLOCK_HELD);`
		`+ return rc;`
		`+ }`
		`+`
		`+ int`
		`+ map_unlock(void)`
		`+ {`
		`+ int lock_status;`
		`+ int lock_count;`
		`+ int rc = 0;`
		`+`
		`+ if (rw_monitor_enabled() == MAP_MONITOR_DISABLED) {`
		`+ /* This is not initialized used the old way */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map_unlock: old way MAP_MONITOR_DISABLED\n");`
		`+ return wrap_rwlock_unlock(map_data.lock);`
		`+ }`
		`+`
		`+ lock_status = get_plugin_monitor_status();`
		`+ lock_count = get_plugin_monitor_count();`
		`+`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "map_unlock",`
		`+ "thread_id = %p (call level = %d)\n", PR_GetCurrentThread(), wrap_get_call_level());`
		`+ #endif`
		`+ if (lock_status == MAP_RWLOCK_UNINIT) {`
		`+ /* This is not initialized used the old way */`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map_unlock: old way lock_status == MAP_RWLOCK_UNINIT\n");`
		`+`
		`+ return wrap_rwlock_unlock(map_data.lock);`
		`+ }`
		`+`
		`+ if (lock_count == 1) {`
		`+ set_plugin_monitor_status(MAP_RWLOCK_FREE);`
		`+ rc = plugin_unlock();`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map unlock: fail to unlock plugin lock (%d)\n", rc);`
		`+ goto common;`
		`+ }`
		`+ #if DEBUG_MAP_LOCK`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map_unlock: (%p) %s --> MAP_RWLOCK_FREE : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD" : (lock_status == MAP_RLOCK_HELD) ? "MAP_RLOCK_HELD" : "MAP_RWLOCK_FREE",`
		`+ 0);`
		`+ #endif`
		`+ rc = wrap_rwlock_unlock(map_data.lock);`
		`+ if (rc) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map_unlock: fail to unlock map lock (%d)\n", rc);`
		`+ goto common;`
		`+ }`
		`+ }`
		`+ if (lock_count >= 1) {`
		`+ set_plugin_monitor_count(lock_count - 1);`
		`+ #if DEBUG_MAP_LOCK`
		`+ if (lock_count > 1) {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map_unlock: (%p) keep %s : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ (lock_status == MAP_WLOCK_HELD) ? "MAP_WLOCK_HELD" : (lock_status == MAP_RLOCK_HELD) ? "MAP_RLOCK_HELD" : "MAP_RWLOCK_FREE",`
		`+ lock_count - 1);`
		`+ } else {`
		`+ slapi_log_error(SLAPI_LOG_FATAL, "schema-compat",`
		`+ "map_unlock: (%p) is now %s : count=%d\n",`
		`+ PR_GetCurrentThread(),`
		`+ "MAP_RWLOCK_FREE",`
		`+ lock_count - 1);`
		`+ }`
		`+ #endif`
		`+ }`
		`+`
		`+ common:`
		`+ return rc;`
		`+ }`

src/map.h

file modified

		`@@ -115,6 +115,9 @@`
		`const char *domain_name);`
		`int map_data_get_map_size(struct plugin_state *state,`
		`const char domain_name, const char map_name);`
		`+ int plugin_rdlock(void);`
		`+ int plugin_wrlock(void);`
		`+ int plugin_unlock(void);`
		`int map_rdlock(void);`
		`int map_wrlock(void);`
		`int map_unlock(void);`

src/plug-sch.c

file modified

+52

		`@@ -62,6 +62,8 @@`
		`#define PLUGIN_ID "schema-compat-plugin"`
		`#define PLUGIN_PREOP_ID PLUGIN_ID "-preop"`
		`#define PLUGIN_BETXN_PREOP_ID PLUGIN_ID "-betxn_preop"`
		`+ #define PLUGIN_BE_POSTOP_ID PLUGIN_ID "-be_postop"`
		`+ #define PLUGIN_BE_PREOP_ID PLUGIN_ID "-be_preop"`
		`#define PLUGIN_BETXN_POSTOP_ID PLUGIN_ID "-betxn_postop"`
		`#define PLUGIN_POSTOP_ID PLUGIN_ID "-postop"`
		`#define PLUGIN_INTERNAL_POSTOP_ID PLUGIN_ID "-internal-postop"`
		`@@ -258,6 +260,35 @@`
		`}`
		`return 0;`
		`}`
		`+ static int`
		`+ schema_compat_plugin_init_bepreop(Slapi_PBlock *pb)`
		`+ {`
		`+ slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_03);`
		`+ slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, &plugin_description);`
		`+ slapi_pblock_set(pb, SLAPI_PLUGIN_PRIVATE, global_plugin_state);`
		`+ if (backend_init_be_preop(pb, global_plugin_state) == -1) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN,`
		`+ global_plugin_state->plugin_desc->spd_id,`
		`+ "error registering be preoperation hooks\n");`
		`+ return -1;`
		`+ }`
		`+ return 0;`
		`+ }`
		`+ static int`
		`+ schema_compat_plugin_init_bepostop(Slapi_PBlock *pb)`
		`+ {`
		`+ slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_03);`
		`+ slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, &plugin_description);`
		`+ slapi_pblock_set(pb, SLAPI_PLUGIN_PRIVATE, global_plugin_state);`
		`+ if (backend_init_be_postop(pb, global_plugin_state) == -1) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN,`
		`+ global_plugin_state->plugin_desc->spd_id,`
		`+ "error registering be postoperation "`
		`+ "hooks\n");`
		`+ return -1;`
		`+ }`
		`+ return 0;`
		`+ }`
		`#endif`
		`static int`
		`schema_compat_plugin_init_postop(Slapi_PBlock *pb)`
		`@@ -300,6 +331,9 @@`
		`"error setting up plugin\n");`
		`return -1;`
		`}`
		`+`
		`+ init_map_lock();`
		`+`
		`/* Read global configuration. */`
		`if ((slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_ENTRY,`
		`&plugin_entry) == 0) &&`
		`@@ -341,6 +375,15 @@`
		`"error registering betxn preoperation plugin\n");`
		`return -1;`
		`}`
		`+ if (slapi_register_plugin("bepreoperation", TRUE,`
		`+ "schema_compat_plugin_init_bepreop",`
		`+ schema_compat_plugin_init_bepreop,`
		`+ PLUGIN_BE_PREOP_ID, NULL,`
		`+ state->plugin_identity) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error registering betxn preoperation plugin\n");`
		`+ return -1;`
		`+ }`
		`#endif`
		`if (slapi_register_plugin("postoperation", TRUE,`
		`"schema_compat_plugin_init_postop",`
		`@@ -370,6 +413,15 @@`
		`"error registering betxn postoperation plugin\n");`
		`return -1;`
		`}`
		`+ if (slapi_register_plugin("bepostoperation", TRUE,`
		`+ "schema_compat_plugin_init_bepostop",`
		`+ schema_compat_plugin_init_bepostop,`
		`+ PLUGIN_BE_POSTOP_ID, NULL,`
		`+ state->plugin_identity) != 0) {`
		`+ slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,`
		`+ "error registering betxn postoperation plugin\n");`
		`+ return -1;`
		`+ }`
		`#endif`
		`if (slapi_register_plugin("preextendedop", TRUE,`
		`"schema_compat_plugin_init_extop",`

tbordaz commented 5 years ago

Problem description:
Schema compat is a betxn pre/post op plugin managing maps.
The maps are protected by a RW lock.
A typical deadlock scenario is when a read thread (SRCH) holding
the map lock needs a ressource (DB page) acquired by an
write thread (MOD/ADD/DEL..) and that write thread needs to update
the map and so acquire the map lock.

So far we have been able to reduce the frequency of those scenarios
(but not eliminate them) by restricting the scope of operations
that need to acquire the lock.

But scoping is not systematically possible like described in
https://bugzilla.redhat.com/show_bug.cgi?id=1435663#c16

Fix description:
The fix implements a plugin RW reentrant lock 'plugin_lock'.
To do this it uses a thread private variables (thread_plugin_lock_status and
thread_plugin_lock_count to remember the current status of the lock
(free, read_acquired, write_acquired).

Then for write operations (even if it is out of the scope of SC), the lock is acquired in write
in the BE_preop operation (note not in BETXN_preop) and only released
in the BE_postop (postop/bepostop).
So a write operation acquires (exclusive) the maps lock before acquiring DB pages.
So read and write operation will acquire locks in the same order.

Design is https://www.freeipa.org/page/V4_slapi_nis_locking

Signed-off-by: Thierry Bordaz tbordaz@redhat.com