[389-commits] [389-ds-base] branch 389-ds-base-1.4.0 updated: Ticket 49873 - (cont 2nd) Contention on virtual attribute lookup

This is an automated email from the git hooks/post-receive script. tbordaz pushed a commit to branch 389-ds-base-1.4.0 in repository 389-ds-base. The following commit(s) were added to refs/heads/389-ds-base-1.4.0 by this push: new 74490fb Ticket 49873 - (cont 2nd) Contention on virtual attribute lookup 74490fb is described below commit 74490fb28c5c091dcc4c6eb492f2cef2acaf4dde Author: Thierry Bordaz <tbordaz(a)redhat.com&gt; AuthorDate: Tue Mar 19 14:17:52 2019 +0100 Ticket 49873 - (cont 2nd) Contention on virtual attribute lookup Bug Description: SSL initialization does internal searches that access the vattr_global_lock Thread private counter needs to be initialized by that time. Currently it is initialized after SSL init. Second problem was a leak of one 'int' per worker. It was used to keep the private counter. Fix Description: Call of vattr_global_lock_create needs to be called before slapd_do_all_nss_ssl_init. Also, 'main' may or may not fork, the initialization fo the thread private variable is done either on the child or parent depending if main forks or not. The leak is fixed using a destructor callback of the private variable and so call PR_SetThreadPrivate only if there is no private variable. https://pagure.io/389-ds-base/issue/49873 Reviewed by: Mark Reynolds, Simon Pichugi (thanks) Platforms tested: F28 Flag Day: no Doc impact: no Ticket foo --- ldap/servers/slapd/detach.c | 9 +++++++++ ldap/servers/slapd/main.c | 4 ---- ldap/servers/slapd/vattr.c | 18 ++++++++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/ldap/servers/slapd/detach.c b/ldap/servers/slapd/detach.c index 681e6a7..d5c95a0 100644 --- a/ldap/servers/slapd/detach.c +++ b/ldap/servers/slapd/detach.c @@ -144,6 +144,10 @@ detach(int slapd_exemode, int importexport_encrypt, int s_port, daemon_ports_t * } break; } + /* The thread private counter needs to be allocated after the fork + * it is not inherited from parent process + */ + vattr_global_lock_create(); /* call this right after the fork, but before closing stdin */ if (slapd_do_all_nss_ssl_init(slapd_exemode, importexport_encrypt, s_port, ports_info)) { @@ -174,6 +178,11 @@ detach(int slapd_exemode, int importexport_encrypt, int s_port, daemon_ports_t * g_set_detached(1); } else { /* not detaching - call nss/ssl init */ + /* The thread private counter needs to be allocated after the fork + * it is not inherited from parent process + */ + vattr_global_lock_create(); + if (slapd_do_all_nss_ssl_init(slapd_exemode, importexport_encrypt, s_port, ports_info)) { return 1; } diff --git a/ldap/servers/slapd/main.c b/ldap/servers/slapd/main.c index 5a86e2e..185ba90 100644 --- a/ldap/servers/slapd/main.c +++ b/ldap/servers/slapd/main.c @@ -950,10 +950,6 @@ main(int argc, char **argv) return_value = 1; goto cleanup; } - /* The thread private counter needs to be allocated after the fork - * it is not inherited from parent process - */ - vattr_global_lock_create(); /* * Create our thread pool here for tasks to utilise. diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c index ce63f50..bc4d0e9 100644 --- a/ldap/servers/slapd/vattr.c +++ b/ldap/servers/slapd/vattr.c @@ -125,11 +125,25 @@ vattr_init() vattr_basic_sp_init(); #endif } +/* + * https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSPR/Reference/... + * It is called each time: + * - PR_SetThreadPrivate is call with a not NULL private value + * - on thread exit + */ +static void +vattr_global_lock_free(void *ptr) +{ + int *nb_acquired = ptr; + if (nb_acquired) { + slapi_ch_free((void **)&nb_acquired); + } +} /* Create a private variable for each individual thread of the current process */ void vattr_global_lock_create() { - if (PR_NewThreadPrivateIndex(&thread_private_global_vattr_lock, NULL) != PR_SUCCESS) { + if (PR_NewThreadPrivateIndex(&thread_private_global_vattr_lock, vattr_global_lock_free) != PR_SUCCESS) { slapi_log_err(SLAPI_LOG_ALERT, "vattr_global_lock_create", "Failure to create global lock for virtual attribute !\n"); PR_ASSERT(0); @@ -155,9 +169,9 @@ global_vattr_lock_set_acquired_count(int nb_acquired) if (val == NULL) { /* if it was not initialized set it to zero */ val = (int *) slapi_ch_calloc(1, sizeof(int)); + PR_SetThreadPrivate(thread_private_global_vattr_lock, (void *) val); } *val = nb_acquired; - PR_SetThreadPrivate(thread_private_global_vattr_lock, (void *) val); } /* The map lock can be acquired recursively. So only the first rdlock * will acquire the lock. -- To stop receiving notification emails like this one, please contact the administrator of this repository.