[Pkg-fedora-ds-maintainers] 389-ds-base: Changes to 'upstream'

Timo Aaltonen tjaalton at moszumanska.debian.org
Tue Dec 12 15:33:33 UTC 2017


Rebased ref, commits from common ancestor:
commit 4234a5f89bda4e3442c232c9c70212b714e97ccf
Author: Mark Reynolds <mreynolds at redhat.com>
Date:   Mon Nov 20 11:16:57 2017 -0500

    Bump version to 1.3.7.8

diff --git a/VERSION.sh b/VERSION.sh
index e41c046..b54771a 100644
--- a/VERSION.sh
+++ b/VERSION.sh
@@ -10,7 +10,7 @@ vendor="389 Project"
 # PACKAGE_VERSION is constructed from these
 VERSION_MAJOR=1
 VERSION_MINOR=3
-VERSION_MAINT=7.7
+VERSION_MAINT=7.8
 # NOTE: VERSION_PREREL is automatically set for builds made out of a git tree
 VERSION_PREREL=
 VERSION_DATE=$(date -u +%Y%m%d)

commit 928e5a4809717f0393c338e22d2aa58ed6d222f4
Author: William Brown <firstyear at redhat.com>
Date:   Fri Nov 17 11:43:36 2017 +1000

    Ticket 49298 - fix complier warn
    
    Bug Description:  Extra argument to error log in dse.c
    
    Fix Description:  Remove extra argument.
    
    https://pagure.io/389-ds-base/issue/49298
    
    Author: wibrown
    
    Review by: oneline rule.

diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c
index 653009f..662e91a 100644
--- a/ldap/servers/slapd/dse.c
+++ b/ldap/servers/slapd/dse.c
@@ -613,7 +613,7 @@ dse_check_file(char *filename, char *backupname)
             return 1;
         } else {
             slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
-                          "The config %s has zero length. Attempting restore ... \n", filename, rc);
+                          "The config %s has zero length. Attempting restore ... \n", filename);
             rc = PR_Delete(filename);
         }
     } else {

commit 3cd1ab4159d14fa52dbe84c95561e328653610cd
Author: William Brown <firstyear at redhat.com>
Date:   Wed Nov 15 13:44:02 2017 +1000

    Ticket 49298 - Correct error codes with config restore.
    
    Bug Description:  The piece of code uses 0 as an error - not 1,
    and in some cases did not even check the codes or use the
    correct logic.
    
    Fix Description:  Cleanup dse_check_file to better check the
    content of files and communicate issues to the admin. Correct
    slapd_bootstrap_config to correctly handle the cases of removal
    and restore.
    
    https://pagure.io/389-ds-base/issue/49298
    
    Author: wibrown
    
    Review by: mreynoolds & spichugi
    
    Signed-off-by: Mark Reynolds <mreynolds at redhat.com>
    (cherry picked from commit 75e55e26579955adf058e8adcba9a28779583b7b)

diff --git a/dirsrvtests/tests/suites/config/removed_config_49298_test.py b/dirsrvtests/tests/suites/config/removed_config_49298_test.py
new file mode 100644
index 0000000..e652369
--- /dev/null
+++ b/dirsrvtests/tests/suites/config/removed_config_49298_test.py
@@ -0,0 +1,81 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2017 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import pytest
+import os
+import logging
+import subprocess
+
+from lib389.topologies import topology_st as topo
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+    logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+def test_restore_config(topo):
+    """
+    Check that if a dse.ldif and backup are removed, that the server still starts.
+
+    :id: e1c38fa7-30bc-46f2-a934-f8336f387581
+    :setup: Standalone instance
+    :steps:
+        1. Stop the instance
+        2. Delete 'dse.ldif'
+        3. Start the instance
+    :expectedresults:
+        1. Steps 1 and 2 succeed.
+        2. Server will succeed to start with restored cfg.
+    """
+    topo.standalone.stop()
+
+    dse_path = topo.standalone.get_config_dir()
+
+    log.info(dse_path)
+
+    for i in ('dse.ldif', 'dse.ldif.startOK'):
+        p = os.path.join(dse_path, i)
+        os.remove(p)
+
+    # This will pass.
+    topo.standalone.start()
+
+def test_removed_config(topo):
+    """
+    Check that if a dse.ldif and backup are removed, that the server
+    exits better than "segfault".
+
+    :id: b45272d1-c197-473e-872f-07257fcb2ec0
+    :setup: Standalone instance
+    :steps:
+        1. Stop the instance
+        2. Delete 'dse.ldif', 'dse.ldif.bak', 'dse.ldif.startOK'
+        3. Start the instance
+    :expectedresults:
+        1. Steps 1 and 2 succeed.
+        2. Server will fail to start, but will not crash.
+    """
+    topo.standalone.stop()
+
+    dse_path = topo.standalone.get_config_dir()
+
+    log.info(dse_path)
+
+    for i in ('dse.ldif', 'dse.ldif.bak', 'dse.ldif.startOK'):
+        p = os.path.join(dse_path, i)
+        os.remove(p)
+
+    # We actually can't check the log output, because it can't read dse.ldif,
+    # don't know where to write it yet! All we want is the server fail to
+    # start here, rather than infinite run + segfault.
+    with pytest.raises(subprocess.CalledProcessError):
+        topo.standalone.start()
+
+
diff --git a/ldap/servers/slapd/config.c b/ldap/servers/slapd/config.c
index afe07df..c8d57e7 100644
--- a/ldap/servers/slapd/config.c
+++ b/ldap/servers/slapd/config.c
@@ -121,14 +121,13 @@ slapd_bootstrap_config(const char *configdir)
                       "Passed null config directory\n");
         return rc; /* Fail */
     }
-    PR_snprintf(configfile, sizeof(configfile), "%s/%s", configdir,
-                CONFIG_FILENAME);
-    PR_snprintf(tmpfile, sizeof(tmpfile), "%s/%s.tmp", configdir,
-                CONFIG_FILENAME);
-    if ((rc = dse_check_file(configfile, tmpfile)) == 0) {
-        PR_snprintf(tmpfile, sizeof(tmpfile), "%s/%s.bak", configdir,
-                    CONFIG_FILENAME);
-        rc = dse_check_file(configfile, tmpfile);
+    PR_snprintf(configfile, sizeof(configfile), "%s/%s", configdir, CONFIG_FILENAME);
+    PR_snprintf(tmpfile, sizeof(tmpfile), "%s/%s.bak", configdir, CONFIG_FILENAME);
+    rc = dse_check_file(configfile, tmpfile);
+    if (rc == 0) {
+        /* EVERYTHING IS GOING WRONG, ARRGHHHHHH */
+        slapi_log_err(SLAPI_LOG_ERR, "slapd_bootstrap_config", "No valid configurations can be accessed! You must restore %s from backup!\n", configfile);
+        return 0;
     }
 
     if ((rc = PR_GetFileInfo64(configfile, &prfinfo)) != PR_SUCCESS) {
diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c
index 420248c..653009f 100644
--- a/ldap/servers/slapd/dse.c
+++ b/ldap/servers/slapd/dse.c
@@ -609,29 +609,49 @@ dse_check_file(char *filename, char *backupname)
 
     if (PR_GetFileInfo64(filename, &prfinfo) == PR_SUCCESS) {
         if (prfinfo.size > 0) {
-            return (1);
+            /* File exists and has content. */
+            return 1;
         } else {
+            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                          "The config %s has zero length. Attempting restore ... \n", filename, rc);
             rc = PR_Delete(filename);
         }
+    } else {
+        slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                      "The config %s can not be accessed. Attempting restore ... (reason: %d)\n", filename, rc);
     }
 
     if (backupname) {
+
+        if (PR_GetFileInfo64(backupname, &prfinfo) != PR_SUCCESS) {
+            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                          "The backup %s can not be accessed. Check it exists and permissions.\n", backupname);
+            return 0;
+        }
+
+        if (prfinfo.size <= 0) {
+            slapi_log_err(SLAPI_LOG_ERR, "dse_check_file",
+                      "The backup file %s has zero length, refusing to restore it.\n", backupname);
+            return 0;
+        }
+
         rc = PR_Rename(backupname, filename);
-    } else {
-        return (0);
-    }
+        if (rc != PR_SUCCESS) {
+            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                      "The configuration file %s was NOT able to be restored from %s, error %d\n", filename, backupname, rc);
+            return 0;
+        }
 
-    if (PR_GetFileInfo64(filename, &prfinfo) == PR_SUCCESS && prfinfo.size > 0) {
         slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
-                      "The configuration file %s was restored from backup %s\n", filename, backupname);
-        return (1);
+                  "The configuration file %s was restored from backup %s\n", filename, backupname);
+        return 1;
+
     } else {
-        slapi_log_err(SLAPI_LOG_ERR, "dse_check_file",
-                      "The configuration file %s was not restored from backup %s, error %d\n",
-                      filename, backupname, rc);
-        return (0);
+        slapi_log_err(SLAPI_LOG_INFO, "dse_check_file", "No backup filename provided.\n");
+        return 0;
     }
 }
+
 static int
 dse_read_one_file(struct dse *pdse, const char *filename, Slapi_PBlock *pb, int primary_file)
 {

commit d67e8aea264bc000c41843b88a03654aa675b04f
Author: William Brown <firstyear at redhat.com>
Date:   Mon Nov 6 08:56:01 2017 +1000

    Ticket 49435 - Fix NS race condition on loaded test systems
    
    Bug Description:  During a test run, on a heavily loaded systems
    some events would time out before they could occur correctly.
    
    Fix Description:  Change the structure of events to mitigate
    a deref performance hit, and add a ns_job_wait conditional
    that allows blocking on a job to complete so that tests do not
    require time based checks.
    
    https://pagure.io/389-ds-base/issue/49435
    
    Author: wibrown
    
    Review by: mreynolds (Thanks!)
    
    (cherry picked from commit 11974a08f7bb083a48590cdc26652934fa74c0cb)

diff --git a/src/nunc-stans/include/nunc-stans.h b/src/nunc-stans/include/nunc-stans.h
index 386a8d2..192e38e 100644
--- a/src/nunc-stans/include/nunc-stans.h
+++ b/src/nunc-stans/include/nunc-stans.h
@@ -77,6 +77,10 @@ typedef enum _ns_result_t {
      * This occurs when a lower level OS issue occurs, generally thread related.
      */
     NS_THREAD_FAILURE = 5,
+    /**
+     * The job is being deleted
+     */
+    NS_DELETING = 6,
 } ns_result_t;
 
 /**
@@ -837,6 +841,14 @@ ns_job_type_t ns_job_get_output_type(struct ns_job_t *job);
 ns_result_t ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func);
 
 /**
+ * Block until a job is completed. This returns the next state of the job as as a return.
+ *
+ * \param job The job to set the callback for.
+ * \retval ns_job_state_t The next state the job will move to. IE, WAITING, DELETED, ARMED.
+ */
+ns_result_t ns_job_wait(struct ns_job_t *job);
+
+/**
  * Creates a new thread pool
  *
  * Must be called with a struct ns_thrpool_config that has been
diff --git a/src/nunc-stans/ns/ns_event_fw.h b/src/nunc-stans/ns/ns_event_fw.h
index 436b282..88997b2 100644
--- a/src/nunc-stans/ns/ns_event_fw.h
+++ b/src/nunc-stans/ns/ns_event_fw.h
@@ -80,7 +80,8 @@ typedef enum _ns_job_state {
    interface between the app/thread pool/event framework */
 typedef struct ns_job_t
 {
-    pthread_mutex_t *monitor;
+    pthread_mutex_t monitor;
+    pthread_cond_t notify;
     struct ns_thrpool_t *tp;
     ns_job_func_t func;
     struct ns_job_data_t *data;
diff --git a/src/nunc-stans/ns/ns_thrpool.c b/src/nunc-stans/ns/ns_thrpool.c
index 2ad0bd7..1d8bb03 100644
--- a/src/nunc-stans/ns/ns_thrpool.c
+++ b/src/nunc-stans/ns/ns_thrpool.c
@@ -214,7 +214,7 @@ job_queue_cleanup(void *arg)
 static void
 internal_ns_job_done(ns_job_t *job)
 {
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "internal_ns_job_done %x state %d moving to NS_JOB_DELETED\n", job, job->state);
 #endif
@@ -239,9 +239,9 @@ internal_ns_job_done(ns_job_t *job)
         job->done_cb(job);
     }
 
-    pthread_mutex_unlock(job->monitor);
-    pthread_mutex_destroy(job->monitor);
-    ns_free(job->monitor);
+    pthread_mutex_unlock(&(job->monitor));
+    pthread_mutex_destroy(&(job->monitor));
+    pthread_cond_destroy(&(job->notify));
 
     ns_free(job);
 }
@@ -250,7 +250,7 @@ internal_ns_job_done(ns_job_t *job)
 static void
 internal_ns_job_rearm(ns_job_t *job)
 {
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state == NS_JOB_NEEDS_ARM);
 /* Don't think I need to check persistence here, it could be the first arm ... */
 #ifdef DEBUG
@@ -267,7 +267,7 @@ internal_ns_job_rearm(ns_job_t *job)
         /* Prevents an un-necessary queue / dequeue to the event_q */
         work_q_notify(job);
     }
-    pthread_mutex_unlock(job->monitor);
+    pthread_mutex_unlock(&(job->monitor));
 }
 
 static void
@@ -281,7 +281,7 @@ work_job_execute(ns_job_t *job)
      * DELETED! Crashes abound, you have been warned ...
      */
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "work_job_execute %x state %d moving to NS_JOB_RUNNING\n", job, job->state);
 #endif
@@ -303,7 +303,12 @@ work_job_execute(ns_job_t *job)
 #ifdef DEBUG
         ns_log(LOG_DEBUG, "work_job_execute %x state %d job func complete, sending to job_done...\n", job, job->state);
 #endif
-        pthread_mutex_unlock(job->monitor);
+        /*
+         * Let waiters know we are done, they'll pick up once
+         * we unlock.
+         */
+        pthread_cond_signal(&(job->notify));
+        pthread_mutex_unlock(&(job->monitor));
         internal_ns_job_done(job);
         /* MUST NOT ACCESS JOB AGAIN.*/
     } else if (job->state == NS_JOB_NEEDS_ARM) {
@@ -311,7 +316,8 @@ work_job_execute(ns_job_t *job)
         ns_log(LOG_DEBUG, "work_job_execute %x state %d job func complete, sending to rearm...\n", job, job->state);
 #endif
         /* Rearm the job! */
-        pthread_mutex_unlock(job->monitor);
+        /* We *don't* notify here because we ARE NOT done! */
+        pthread_mutex_unlock(&(job->monitor));
         internal_ns_job_rearm(job);
     } else {
 #ifdef DEBUG
@@ -321,7 +327,12 @@ work_job_execute(ns_job_t *job)
         PR_ASSERT(!NS_JOB_IS_PERSIST(job->job_type));
         /* We are now idle, set waiting. */
         job->state = NS_JOB_WAITING;
-        pthread_mutex_unlock(job->monitor);
+        /*
+         * Let waiters know we are done, they'll pick up once
+         * we unlock.
+         */
+        pthread_cond_signal(&(job->notify));
+        pthread_mutex_unlock(&(job->monitor));
     }
     /* MUST NOT ACCESS JOB AGAIN */
 }
@@ -338,7 +349,7 @@ static void
 work_q_notify(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "work_q_notify %x state %d\n", job, job->state);
 #endif
@@ -346,12 +357,12 @@ work_q_notify(ns_job_t *job)
     if (job->state != NS_JOB_ARMED) {
         /* Maybe we should return some error here? */
         ns_log(LOG_ERR, "work_q_notify %x state %d is not ARMED, cannot queue!\n", job, job->state);
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return;
     }
     /* MUST NOT ACCESS job after enqueue. So we stash tp.*/
     ns_thrpool_t *ltp = job->tp;
-    pthread_mutex_unlock(job->monitor);
+    pthread_mutex_unlock(&(job->monitor));
     sds_lqueue_enqueue(ltp->work_q, (void *)job);
     pthread_mutex_lock(&(ltp->work_q_lock));
     pthread_cond_signal(&(ltp->work_q_cv));
@@ -411,13 +422,13 @@ static void
 update_event(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "update_event %x state %d\n", job, job->state);
 #endif
     PR_ASSERT(job->state == NS_JOB_NEEDS_DELETE || job->state == NS_JOB_ARMED);
     if (job->state == NS_JOB_NEEDS_DELETE) {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         internal_ns_job_done(job);
         return;
     } else if (NS_JOB_IS_IO(job->job_type) || job->ns_event_fw_fd) {
@@ -426,7 +437,7 @@ update_event(ns_job_t *job)
         } else {
             job->tp->ns_event_fw->ns_event_fw_mod_io(job->tp->ns_event_fw_ctx, job);
         }
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         /* We need these returns to prevent a race on the next else if condition when we release job->monitor */
         return;
     } else if (NS_JOB_IS_TIMER(job->job_type) || job->ns_event_fw_time) {
@@ -435,7 +446,7 @@ update_event(ns_job_t *job)
         } else {
             job->tp->ns_event_fw->ns_event_fw_mod_timer(job->tp->ns_event_fw_ctx, job);
         }
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return;
     } else if (NS_JOB_IS_SIGNAL(job->job_type) || job->ns_event_fw_sig) {
         if (!job->ns_event_fw_sig) {
@@ -443,15 +454,15 @@ update_event(ns_job_t *job)
         } else {
             job->tp->ns_event_fw->ns_event_fw_mod_signal(job->tp->ns_event_fw_ctx, job);
         }
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return;
     } else {
         /* It's a "run now" job. */
         if (NS_JOB_IS_THREAD(job->job_type)) {
-            pthread_mutex_unlock(job->monitor);
+            pthread_mutex_unlock(&(job->monitor));
             work_q_notify(job);
         } else {
-            pthread_mutex_unlock(job->monitor);
+            pthread_mutex_unlock(&(job->monitor));
             event_q_notify(job);
         }
     }
@@ -602,14 +613,14 @@ event_cb(ns_job_t *job)
      */
 
     /* There is no guarantee this won't be called once we start to enter the shutdown, especially with timers .... */
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 
     PR_ASSERT(job->state == NS_JOB_ARMED || job->state == NS_JOB_NEEDS_DELETE);
     if (job->state == NS_JOB_ARMED && NS_JOB_IS_THREAD(job->job_type)) {
 #ifdef DEBUG
         ns_log(LOG_DEBUG, "event_cb %x state %d threaded, send to work_q\n", job, job->state);
 #endif
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         work_q_notify(job);
     } else if (job->state == NS_JOB_NEEDS_DELETE) {
 #ifdef DEBUG
@@ -620,14 +631,14 @@ event_cb(ns_job_t *job)
          * It's here because it's been QUEUED for deletion and *may* be coming
          * from the thrpool destroy thread!
          */
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
 
     } else {
 #ifdef DEBUG
         ns_log(LOG_DEBUG, "event_cb %x state %d non-threaded, execute right meow\n", job, job->state);
 #endif
         /* Not threaded, execute now! */
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         work_job_execute(job);
         /* MUST NOT ACCESS JOB FROM THIS POINT */
     }
@@ -682,12 +693,12 @@ static ns_job_t *
 new_ns_job(ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, ns_job_func_t func, struct ns_job_data_t *data)
 {
     ns_job_t *job = ns_calloc(1, sizeof(ns_job_t));
-    job->monitor = ns_calloc(1, sizeof(pthread_mutex_t));
 
     pthread_mutexattr_t *monitor_attr = ns_calloc(1, sizeof(pthread_mutexattr_t));
     pthread_mutexattr_init(monitor_attr);
     pthread_mutexattr_settype(monitor_attr, PTHREAD_MUTEX_RECURSIVE);
-    assert(pthread_mutex_init(job->monitor, monitor_attr) == 0);
+    assert(pthread_mutex_init(&(job->monitor), monitor_attr) == 0);
+    assert(pthread_cond_init(&(job->notify), NULL) == 0);
     ns_free(monitor_attr);
 
     job->tp = tp;
@@ -746,14 +757,14 @@ ns_job_done(ns_job_t *job)
     /* Get the shutdown state ONCE at the start, atomically */
     int32_t shutdown_state = ns_thrpool_is_shutdown(job->tp);
 
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 
     if (job->state == NS_JOB_NEEDS_DELETE || job->state == NS_JOB_DELETED) {
 /* Just return if the job has been marked for deletion */
 #ifdef DEBUG
         ns_log(LOG_DEBUG, "ns_job_done %x tp shutdown -> %x state %d return early\n", job, shutdown_state, job->state);
 #endif
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_SUCCESS;
     }
 
@@ -762,7 +773,7 @@ ns_job_done(ns_job_t *job)
 #ifdef DEBUG
         ns_log(LOG_DEBUG, "ns_job_done %x tp shutdown -> false state %d failed to mark as done\n", job, job->state);
 #endif
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_INVALID_STATE;
     }
 
@@ -773,13 +784,13 @@ ns_job_done(ns_job_t *job)
         ns_log(LOG_DEBUG, "ns_job_done %x tp shutdown -> false state %d setting to async NS_JOB_NEEDS_DELETE\n", job, job->state);
 #endif
         job->state = NS_JOB_NEEDS_DELETE;
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
     } else if (!shutdown_state) {
 #ifdef DEBUG
         ns_log(LOG_DEBUG, "ns_job_done %x tp shutdown -> false state %d setting NS_JOB_NEEDS_DELETE and queuing\n", job, job->state);
 #endif
         job->state = NS_JOB_NEEDS_DELETE;
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         event_q_notify(job);
     } else {
 #ifdef DEBUG
@@ -787,7 +798,7 @@ ns_job_done(ns_job_t *job)
 #endif
         job->state = NS_JOB_NEEDS_DELETE;
         /* We are shutting down, just remove it! */
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         internal_ns_job_done(job);
     }
     return NS_SUCCESS;
@@ -849,12 +860,12 @@ ns_add_io_job(ns_thrpool_t *tp, PRFileDesc *fd, ns_job_type_t job_type, ns_job_f
         return NS_ALLOCATION_FAILURE;
     }
 
-    pthread_mutex_lock(_job->monitor);
+    pthread_mutex_lock(&(_job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "ns_add_io_job state %d moving to NS_JOB_ARMED\n", (_job)->state);
 #endif
     _job->state = NS_JOB_NEEDS_ARM;
-    pthread_mutex_unlock(_job->monitor);
+    pthread_mutex_unlock(&(_job->monitor));
     internal_ns_job_rearm(_job);
 
     /* fill in a pointer to the job for the caller if requested */
@@ -889,12 +900,12 @@ ns_add_timeout_job(ns_thrpool_t *tp, struct timeval *tv, ns_job_type_t job_type,
         return NS_ALLOCATION_FAILURE;
     }
 
-    pthread_mutex_lock(_job->monitor);
+    pthread_mutex_lock(&(_job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "ns_add_timeout_job state %d moving to NS_JOB_ARMED\n", (_job)->state);
 #endif
     _job->state = NS_JOB_NEEDS_ARM;
-    pthread_mutex_unlock(_job->monitor);
+    pthread_mutex_unlock(&(_job->monitor));
     internal_ns_job_rearm(_job);
 
     /* fill in a pointer to the job for the caller if requested */
@@ -944,14 +955,14 @@ ns_add_io_timeout_job(ns_thrpool_t *tp, PRFileDesc *fd, struct timeval *tv, ns_j
     if (!_job) {
         return NS_ALLOCATION_FAILURE;
     }
-    pthread_mutex_lock(_job->monitor);
+    pthread_mutex_lock(&(_job->monitor));
     _job->tv = *tv;
 
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "ns_add_io_timeout_job state %d moving to NS_JOB_ARMED\n", (_job)->state);
 #endif
     _job->state = NS_JOB_NEEDS_ARM;
-    pthread_mutex_unlock(_job->monitor);
+    pthread_mutex_unlock(&(_job->monitor));
     internal_ns_job_rearm(_job);
 
     /* fill in a pointer to the job for the caller if requested */
@@ -982,12 +993,12 @@ ns_add_signal_job(ns_thrpool_t *tp, int32_t signum, ns_job_type_t job_type, ns_j
         return NS_ALLOCATION_FAILURE;
     }
 
-    pthread_mutex_lock(_job->monitor);
+    pthread_mutex_lock(&(_job->monitor));
 #ifdef DEBUG
     ns_log(LOG_DEBUG, "ns_add_signal_job state %d moving to NS_JOB_ARMED\n", (_job)->state);
 #endif
     _job->state = NS_JOB_NEEDS_ARM;
-    pthread_mutex_unlock(_job->monitor);
+    pthread_mutex_unlock(&(_job->monitor));
     internal_ns_job_rearm(_job);
 
     /* fill in a pointer to the job for the caller if requested */
@@ -1038,9 +1049,9 @@ ns_add_shutdown_job(ns_thrpool_t *tp)
     if (!_job) {
         return NS_ALLOCATION_FAILURE;
     }
-    pthread_mutex_lock(_job->monitor);
+    pthread_mutex_lock(&(_job->monitor));
     _job->state = NS_JOB_NEEDS_ARM;
-    pthread_mutex_unlock(_job->monitor);
+    pthread_mutex_unlock(&(_job->monitor));
     internal_ns_job_rearm(_job);
     return NS_SUCCESS;
 }
@@ -1061,13 +1072,13 @@ void *
 ns_job_get_data(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state != NS_JOB_DELETED);
     if (job->state != NS_JOB_DELETED) {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return job->data;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NULL;
     }
 }
@@ -1076,14 +1087,14 @@ ns_result_t
 ns_job_set_data(ns_job_t *job, void *data)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING);
     if (job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING) {
         job->data = data;
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_SUCCESS;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_INVALID_STATE;
     }
 }
@@ -1092,13 +1103,13 @@ ns_thrpool_t *
 ns_job_get_tp(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state != NS_JOB_DELETED);
     if (job->state != NS_JOB_DELETED) {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return job->tp;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NULL;
     }
 }
@@ -1107,13 +1118,13 @@ ns_job_type_t
 ns_job_get_output_type(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state == NS_JOB_RUNNING);
     if (job->state == NS_JOB_RUNNING) {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return job->output_job_type;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return 0;
     }
 }
@@ -1122,13 +1133,13 @@ ns_job_type_t
 ns_job_get_type(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state != NS_JOB_DELETED);
     if (job->state != NS_JOB_DELETED) {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return job->job_type;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return 0;
     }
 }
@@ -1137,13 +1148,13 @@ PRFileDesc *
 ns_job_get_fd(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state != NS_JOB_DELETED);
     if (job->state != NS_JOB_DELETED) {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return job->fd;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NULL;
     }
 }
@@ -1152,18 +1163,40 @@ ns_result_t
 ns_job_set_done_cb(struct ns_job_t *job, ns_job_func_t func)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING);
     if (job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING) {
         job->done_cb = func;
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_SUCCESS;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_INVALID_STATE;
     }
 }
 
+ns_result_t
+ns_job_wait(struct ns_job_t *job) {
+    PR_ASSERT(job);
+    pthread_mutex_lock(&(job->monitor));
+    if (job->state == NS_JOB_WAITING) {
+        /* It's done */
+        pthread_mutex_unlock(&(job->monitor));
+        return NS_SUCCESS;
+    } else {
+        pthread_cond_wait(&(job->notify), &(job->monitor));
+        ns_job_state_t result = job->state;
+        pthread_mutex_unlock(&(job->monitor));
+        if (result == NS_JOB_WAITING) {
+            return NS_SUCCESS;
+        } else if (result == NS_JOB_NEEDS_DELETE) {
+            return NS_DELETING;
+        } else {
+            PR_ASSERT(1 == 0);
+            return NS_INVALID_STATE;
+        }
+    }
+}
 
 /*
  * This is a convenience function - use if you need to re-arm the same event
@@ -1173,7 +1206,7 @@ ns_result_t
 ns_job_rearm(ns_job_t *job)
 {
     PR_ASSERT(job);
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
     PR_ASSERT(job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING);
 
     if (ns_thrpool_is_shutdown(job->tp)) {
@@ -1186,7 +1219,7 @@ ns_job_rearm(ns_job_t *job)
 #endif
         job->state = NS_JOB_NEEDS_ARM;
         internal_ns_job_rearm(job);
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_SUCCESS;
     } else if (!NS_JOB_IS_PERSIST(job->job_type) && job->state == NS_JOB_RUNNING) {
 /* For this to be called, and NS_JOB_RUNNING, we *must* be the callback thread! */
@@ -1195,10 +1228,10 @@ ns_job_rearm(ns_job_t *job)
         ns_log(LOG_DEBUG, "ns_rearm_job %x state %d setting NS_JOB_NEEDS_ARM\n", job, job->state);
 #endif
         job->state = NS_JOB_NEEDS_ARM;
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_SUCCESS;
     } else {
-        pthread_mutex_unlock(job->monitor);
+        pthread_mutex_unlock(&(job->monitor));
         return NS_INVALID_STATE;
     }
     /* Unreachable code .... */
@@ -1254,7 +1287,7 @@ setup_event_q_wakeup(ns_thrpool_t *tp)
                            NS_JOB_READ | NS_JOB_PERSIST | NS_JOB_PRESERVE_FD,
                            wakeup_cb, NULL);
 
-    pthread_mutex_lock(job->monitor);
+    pthread_mutex_lock(&(job->monitor));
 
 /* The event_queue wakeup is ready, arm it. */
 #ifdef DEBUG
@@ -1267,7 +1300,7 @@ setup_event_q_wakeup(ns_thrpool_t *tp)
 
     /* Stash the wakeup job in tp so we can release it later. */
     tp->event_q_wakeup_job = job;
-    pthread_mutex_unlock(job->monitor);
+    pthread_mutex_unlock(&(job->monitor));
 }
 
 /* Initialize the thrpool config */
@@ -1463,7 +1496,7 @@ ns_thrpool_destroy(struct ns_thrpool_t *tp)
          * and use it to wake up the event loop.
          */
 
-        pthread_mutex_lock(tp->event_q_wakeup_job->monitor);
+        pthread_mutex_lock(&(tp->event_q_wakeup_job->monitor));
 
 // tp->event_q_wakeup_job->job_type |= NS_JOB_THREAD;
 /* This triggers the job to "run", which will cause a shutdown cascade */
@@ -1471,7 +1504,7 @@ ns_thrpool_destroy(struct ns_thrpool_t *tp)
         ns_log(LOG_DEBUG, "ns_thrpool_destroy %x state %d moving to NS_JOB_NEEDS_DELETE\n", tp->event_q_wakeup_job, tp->event_q_wakeup_job->state);
 #endif
         tp->event_q_wakeup_job->state = NS_JOB_NEEDS_DELETE;
-        pthread_mutex_unlock(tp->event_q_wakeup_job->monitor);
+        pthread_mutex_unlock(&(tp->event_q_wakeup_job->monitor));
         /* Has to be event_q_notify, not internal_job_done */
         event_q_notify(tp->event_q_wakeup_job);
 
diff --git a/src/nunc-stans/test/test_nuncstans.c b/src/nunc-stans/test/test_nuncstans.c
index 629377a..afe3c02 100644
--- a/src/nunc-stans/test/test_nuncstans.c
+++ b/src/nunc-stans/test/test_nuncstans.c
@@ -55,14 +55,21 @@
 /* We need the internal headers for state checks */
 #include "../ns/ns_event_fw.h"
 
+#include <assert.h>
+
+#include <time.h>
+
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
 #endif
 
 
 static int cb_check = 0;
-static PRLock *cb_lock = NULL;
-static PRCondVar *cb_cond = NULL;
+
+static pthread_mutex_t cb_lock;
+static pthread_cond_t cb_cond;
+// static PRLock *cb_lock = NULL;
+// static PRCondVar *cb_cond = NULL;
 
 void
 ns_test_logger(int priority __attribute__((unused)), const char *fmt, va_list varg)
@@ -71,6 +78,19 @@ ns_test_logger(int priority __attribute__((unused)), const char *fmt, va_list va
     vprintf(fmt, varg);
 }
 
+static int
+cond_wait_rel(pthread_cond_t *restrict cond, pthread_mutex_t *restrict mutex, const struct timespec *restrict reltime) {
+    struct timespec now;
+    struct timespec abswait;
+
+    clock_gettime(CLOCK_REALTIME, &now);
+
+    abswait.tv_sec = now.tv_sec + reltime->tv_sec;
+    abswait.tv_nsec = now.tv_nsec + reltime->tv_nsec;
+
+    return pthread_cond_timedwait(cond, mutex, &abswait);
+}
+
 /* All our other tests will use this in some form. */
 static int
 ns_test_setup(void **state)
@@ -81,8 +101,8 @@ ns_test_setup(void **state)
     /* Reset the callback check */
     cb_check = 0;
     /* Create the cond var the CB check will use. */
-    cb_lock = PR_NewLock();
-    cb_cond = PR_NewCondVar(cb_lock);
+    assert(pthread_mutex_init(&cb_lock, NULL) == 0);
+    assert(pthread_cond_init(&cb_cond, NULL) == 0);
 
     ns_thrpool_config_init(&ns_config);
 
@@ -105,8 +125,8 @@ ns_test_teardown(void **state)
 
     ns_thrpool_destroy(tp);
 
-    PR_DestroyCondVar(cb_cond);
-    PR_DestroyLock(cb_lock);
+    pthread_cond_destroy(&cb_cond);
+    pthread_mutex_destroy(&cb_lock);
 
     return 0;
 }
@@ -114,24 +134,23 @@ ns_test_teardown(void **state)
 static void
 ns_init_test_job_cb(struct ns_job_t *job __attribute__((unused)))
 {
+    pthread_mutex_lock(&cb_lock);
     cb_check += 1;
-    PR_Lock(cb_lock);
-    PR_NotifyCondVar(cb_cond);
-    PR_Unlock(cb_lock);
+    pthread_cond_signal(&cb_cond);
+    pthread_mutex_unlock(&cb_lock);
 }
 
 static void
 ns_init_disarm_job_cb(struct ns_job_t *job)
 {
     if (ns_job_done(job) == NS_SUCCESS) {
+        pthread_mutex_lock(&cb_lock);
         cb_check = 1;
+        pthread_cond_signal(&cb_cond);
+        pthread_mutex_unlock(&cb_lock);
     } else {
         assert_int_equal(1, 0);
     }
-    PR_Lock(cb_lock);
-    PR_NotifyCondVar(cb_cond);
-    /* Disarm ourselves */
-    PR_Unlock(cb_lock);
 }
 
 static void
@@ -146,20 +165,20 @@ ns_init_test(void **state)
 {
     struct ns_thrpool_t *tp = *state;
     struct ns_job_t *job = NULL;
+    struct timespec timeout = {1, 0};
 
-    PR_Lock(cb_lock);
+    pthread_mutex_lock(&cb_lock);
     assert_int_equal(
         ns_add_job(tp, NS_JOB_NONE | NS_JOB_THREAD, ns_init_test_job_cb, NULL, &job),
         0);
 
-    PR_WaitCondVar(cb_cond, PR_SecondsToInterval(1));
-    PR_Unlock(cb_lock);
+    assert(cond_wait_rel(&cb_cond, &cb_lock, &timeout) == 0);
+    pthread_mutex_unlock(&cb_lock);
 
     assert_int_equal(cb_check, 1);
 
     /* Once the job is done, it's not in the event queue, and it's complete */
-    /* We have to stall momentarily to let the work_job_execute release the job to us */
-    PR_Sleep(PR_SecondsToInterval(1));
+    assert(ns_job_wait(job) == NS_SUCCESS);



More information about the Pkg-fedora-ds-maintainers mailing list