On cleanup call thd_cleanup function for pool_t objects (#2836)

* cleanup pool_t objects via thd_cleanup: while not a leak, this frees more than
  660KB on exit and makes it easier to identify next items to cleanup
* break down gbl_thd_linger cond_wait to one second chunks
  and break/exit as early as possible
* tests setup: append test lrl last to take precedence
* pthread_attr_setstacksize will fail if size < PTHREAD_STACK_MIN
* set gbl_db_is_exiting to timestamp of exiting

Signed-off-by: Adi Zaimi <azaimi@bloomberg.net>

bdb/file.c

@@ -1248,9 +1248,10 @@ int bdb_del_list_free(void *list, int *bdberr)
     listc_t *list_ptr = list;
 
     /* free each item */
-    LISTC_FOR_EACH_SAFE(list_ptr, item, tmp, lnk)
-    /* remove and free item */
-    free(listc_rfl(list, item));
+    LISTC_FOR_EACH_SAFE(list_ptr, item, tmp, lnk) {
+        /* remove and free item */
+        free(listc_rfl(list, item));
+    }
 
     listc_free(list);
 

db/comdb2.c

@@ -337,7 +337,7 @@ int gbl_meta_lite = 1;
 int gbl_context_in_key = 1;
 int gbl_ready = 0; /* gets set just before waitft is called
                       and never gets unset */
-int gbl_db_is_exiting = 0; /* Indicates this process is exiting */
+static int gbl_db_is_exiting = 0; /* Indicates this process is exiting */
 
 
 int gbl_debug_omit_dta_write;
@@ -1049,7 +1049,7 @@ int db_is_stopped(void)
 
 int db_is_exiting()
 {
-    return ATOMIC_LOAD32(gbl_db_is_exiting);
+    return ATOMIC_LOAD32(gbl_db_is_exiting) != 0;
 }
 
 void print_dbsize(void);
@@ -1567,6 +1567,7 @@ static void finish_clean()
     free_tzdir();
     tz_hash_free();
     clear_sqlhist();
+    thd_cleanup();
     if(!all_string_references_cleared())
         abort();
 }
@@ -1593,7 +1594,7 @@ static void begin_clean_exit(void)
     /* this defaults to 5 minutes */
     alarm(alarmtime);
 
-    XCHANGE32(gbl_db_is_exiting, 1);
+    XCHANGE32(gbl_db_is_exiting, comdb2_time_epoch());
 
     /* dont let any new requests come in.  we're going to go non-coherent
        here in a second, so letting new reads in would be bad. */

db/comdb2.h

@@ -1809,6 +1809,7 @@ extern int gbl_test_io_errors;
 /* init routines */
 int appsock_init(void);
 int thd_init(void);
+void thd_cleanup();
 void sqlinit(void);
 void sqlnet_init(void);
 int clnt_stats_init(void);

db/handle_buf.c

@@ -138,6 +138,42 @@ static void thd_io_complete(void)
     UNLOCK(&lock);
 }
 
+
+#define LISTC_CLEAN(listp, lnk, dofree, type) { \
+    type *item, *tmp; \
+    /* free each item */ \
+    LISTC_FOR_EACH_SAFE(listp, item, tmp, lnk) { \
+        /* remove and potentially free item */ \
+        listc_rfl(listp, item); \
+        if (dofree) \
+            free(item); \
+    } \
+}
+
+
+void thd_cleanup()
+{
+    int counter = 0;
+    while (nthdcreates > nretire && counter++ < gbl_thd_linger)
+        sleep(1);
+
+    if (nthdcreates > nretire)
+        abort();
+
+    LISTC_CLEAN(&busy, lnk, 0, struct thd);
+    LISTC_CLEAN(&idle, lnk, 0, struct thd);
+    LISTC_CLEAN(&rq_reqs, rqlnk, 0, struct dbq_entry_t);
+    LISTC_CLEAN(&q_reqs, qlnk, 0, struct dbq_entry_t);
+    pool_clear(pq_reqs);
+    pool_clear(p_slocks);
+    pool_clear(p_bufs);
+    pool_clear(p_reqs);
+    pool_clear(p_thds);
+    Pthread_cond_destroy(&coalesce_wakeup);
+    Pthread_attr_destroy(&attr);
+    Pthread_mutex_destroy(&lock);
+}
+
 int thd_init(void)
 {
     Pthread_mutex_init(&lock, 0);
@@ -602,16 +638,19 @@ static void *thd_req(void *vthd)
                     memset(&ts, 0, sizeof(ts)); /*force failure later*/
                 }
 
-                ts.tv_sec += gbl_thd_linger;
                 rc = 0;
-                do {
+                int ii = 0;
+                do { /* wait gbl_thd_linger seconds via one second increments */
+                    ts.tv_sec += 1;
+                    ii++;
                     /*waitft thread will deposit a request in thd->iq*/
                     rc = pthread_cond_timedwait(&thd->wakeup, &lock, &ts);
-                } while (thd->iq == 0 && rc == 0);
+                } while ((thd->iq == 0 && rc == 0) ||
+                         (rc == ETIMEDOUT && ii < gbl_thd_linger && !db_is_exiting()));
+
                 if (rc != 0 && rc != ETIMEDOUT) {
-                    logmsg(LOGMSG_ERROR, "thd_req:pthread_cond_timedwait "
-                                    "failed:%s\n",
-                            strerror(rc));
+                    logmsg(LOGMSG_ERROR, "thd_req:pthread_cond_timedwait failed:%s\n",
+                           strerror(rc));
                     /* error'd out, so i still have lock: errLOCK(&lock);*/
                 }
                 if (thd->iq == 0) /*nothing to do. this thread retires.*/
@@ -1016,8 +1055,7 @@ int handle_buf_main2(struct dbenv *dbenv, SBUF2 *sb, const uint8_t *p_buf,
                     if (nextrq == NULL)
                         break;
                     iq = nextrq->obj;
-                    /* remove from global list, and release link block of
-                     * reader*/
+                    /* remove from global list, and release link block of reader*/
                     listc_rfl(&q_reqs, nextrq);
                     if (add_latency > 0) {
                         poll(0, 0, rand() % add_latency);
@@ -1051,8 +1089,7 @@ int handle_buf_main2(struct dbenv *dbenv, SBUF2 *sb, const uint8_t *p_buf,
                         break;
                     }
                 }
-                if ((thd = listc_rtl(&idle)) !=
-                    NULL) /*try to find an idle thread*/
+                if ((thd = listc_rtl(&idle)) != NULL) /*try to find an idle thread*/
                 {
 #if 0
                 printf("%s:%d: thdpool FOUND THD=%p -> newTHD=%d iq=%p\n", __func__, __LINE__, pthread_self(), thd->tid, iq);

tests/grace_period_on_exit.test/lrl.options

@@ -0,0 +1 @@
+exitalarmsec 100

tests/logfill.test/Makefile

@@ -4,5 +4,5 @@ else
 	include $(TESTSROOTDIR)/testcase.mk
 endif
 ifeq ($(TEST_TIMEOUT),)
-	export TEST_TIMEOUT=15m
+	export TEST_TIMEOUT=20m
 endif

tests/setup

@@ -99,14 +99,6 @@ setup_db() {
     fi
     echo "eventlog_nkeep 0" >> ${LRL}
 
-    if [[ -f lrl ]]; then
-        cat lrl >> ${LRL}
-    fi
-    if [[ -f lrl.options ]]; then
-        # use sed to substitute in test specific variables into lrl
-        cat lrl.options | sed "s#\${TESTDIR}#${TESTDIR}#" >> ${LRL}
-    fi
-
     echo -e "name    ${DBNAME}\ndir     ${DBDIR}\n\nsetattr MASTER_REJECT_REQUESTS 0" >> ${LRL}
     echo -e "forbid_remote_admin off" >> ${LRL}
 
@@ -146,6 +138,15 @@ setup_db() {
         echo "portmux_bind_path $pmux_socket" >> ${LRL}
     fi
 
+    # let test specific lrl options take precedence
+    if [[ -f lrl ]]; then
+        cat lrl >> ${LRL}
+    fi
+    if [[ -f lrl.options ]]; then
+        # use sed to substitute in test specific variables into lrl
+        cat lrl.options | sed "s#\${TESTDIR}#${TESTDIR}#" >> ${LRL}
+    fi
+
     SSH_OPT="-o StrictHostKeyChecking=no "
 
     for csc2 in $(ls *.csc2 2>/dev/null); do

tools/cdb2sockpool/cdb2sockpool.c

@@ -30,6 +30,7 @@
 
 #include <errno.h>
 #include <pthread.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -144,7 +145,7 @@ static int pthread_create_attrs(pthread_t *tid, int detachstate,
         tid = &local_tid;
     Pthread_attr_init(&attr);
 
-    if (stacksize > 0) {
+    if (stacksize > PTHREAD_STACK_MIN) {
         Pthread_attr_setstacksize(&attr, stacksize);
     }
     rc = pthread_attr_setdetachstate(&attr, detachstate);

util/averager.c

@@ -55,7 +55,8 @@ struct averager *averager_new(int limit, int maxpoints)
     return avg;
 }
 
-void averager_clear(struct averager *avg) {
+void averager_clear(struct averager *avg)
+{
     struct tick *t;
     t = listc_rtl(&avg->ticks);
     while (t) {