[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.