From 38d4e5231bebcd03715abec7a67caa33161dc5b0 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz <tbordaz@redhat.com>
Date: Mar 27 2019 09:28:52 +0000
Subject: 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

---

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/PR_NewThreadPrivateIndex
+ * 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.