clean up schedule exit code

because of back & forth signaling issues, we have to be careful
how we clean up memory.

First, the network is signaled to exit.  When it does, the
scheduler thread sends a semaphore saying so.  That signals the
master thread to call fr_network_destroy(), which in turn signals
the worker threads that it's exiting.

BUT fr_network_destory() doesn't free underlying memory.  Instead,
we have to wait for all of the workers to finish.

The workers are signaled to finish, and again don't call
fr_worker_destroy().  Instead, they don't ACK the channel close,
but do signal the semaphore again.

When all of the workers are done, it's safe to free the network
memory.  And then it's also safe to free the worker memory.

... and then disable most of it, because apparently even
signaling the workers to exit causes problems.

diff --git a/src/lib/io/network.c b/src/lib/io/network.c
index 29305b5ab1a163e48291bbabbfdb2564711263c9..a95c76cfca35dc564c4f372bd12461dd5119a991 100644 (file)
--- a/src/lib/io/network.c
+++ b/src/lib/io/network.c
@@ -934,7 +934,9 @@ int fr_network_destroy(fr_network_t *nr)
 
        (void) fr_event_post_delete(nr->el, fr_network_post_event, nr);
 
-       talloc_free(nr);
+       /*
+        *      The caller has to free 'nr'.
+        */
 
        return 0;
 }

diff --git a/src/lib/io/schedule.c b/src/lib/io/schedule.c
index 1325dc0a5b17f663e6a342778fea9f10a429e382..d03a7b1cafe25501c219c60524755827fb1b9b4d 100644 (file)
--- a/src/lib/io/schedule.c
+++ b/src/lib/io/schedule.c
@@ -76,6 +76,7 @@ typedef enum fr_schedule_child_status_t {
  *     A data structure to track workers.
  */
 typedef struct fr_schedule_worker_t {
+       TALLOC_CTX      *ctx;                   //!< our allocation ctx
        pthread_t       pthread_id;             //!< the thread of this worker
 
        int             id;                     //!< a unique ID
@@ -94,6 +95,7 @@ typedef struct fr_schedule_worker_t {
  *     A data structure to track network threads / networks.
  */
 typedef struct fr_schedule_network_t {
+       TALLOC_CTX      *ctx;                   //!< our allocation ctx
        pthread_t       pthread_id;             //!< the thread of this network
 
        int             id;                     //!< a unique ID
@@ -144,22 +146,29 @@ struct fr_schedule_t {
  */
 static void *fr_schedule_worker_thread(void *arg)
 {
+       TALLOC_CTX *ctx;
        fr_schedule_worker_t *sw = arg;
        fr_schedule_t *sc = sw->sc;
        fr_schedule_child_status_t status = FR_CHILD_FAIL;
        fr_event_list_t *el;
        char buffer[32];
 
+       sw->ctx = ctx = talloc_init("worker %d", sw->id);
+       if (!ctx) {
+               fr_log(sc->log, L_ERR, "Worker %d - Failed allocating memory", sw->id);
+               goto fail;
+       }
+
        fr_log(sc->log, L_INFO, "Worker %d starting\n", sw->id);
 
-       el = fr_event_list_alloc(sw, NULL, NULL);
+       el = fr_event_list_alloc(ctx, NULL, NULL);
        if (!el) {
                fr_log(sc->log, L_ERR, "Worker %d - Failed creating event list: %s",
                       sw->id, fr_strerror());
                goto fail;
        }
 
-       sw->worker = fr_worker_create(sw, el, sc->log, sc->lvl);
+       sw->worker = fr_worker_create(ctx, el, sc->log, sc->lvl);
        if (!sw->worker) {
                fr_log(sc->log, L_ERR, "Worker %d - Failed creating worker: %s", sw->id, fr_strerror());
                goto fail;
@@ -197,17 +206,10 @@ static void *fr_schedule_worker_thread(void *arg)
 
        fr_log(sc->log, L_INFO, "Worker %d finished\n", sw->id);
 
-       /*
-        *      Talloc ordering issues. We want to be independent of
-        *      how talloc walks it's children, and ensure that some
-        *      things are freed in a specific order.
-        */
-       fr_worker_destroy(sw->worker);
-       sw->worker = NULL;
-
        status = FR_CHILD_EXITED;
 
 fail:
+       if (sw->ctx) TALLOC_FREE(sw->ctx);
 
        sw->status = status;
 
@@ -237,7 +239,7 @@ static void *fr_schedule_network_thread(void *arg)
 
        fr_log(sc->log, L_INFO, "Network %d starting\n", sn->id);
 
-       ctx = talloc_init("network %d", sn->id);
+       sn->ctx = ctx = talloc_init("network %d", sn->id);
        if (!ctx) {
                fr_log(sc->log, L_ERR, "Network %d - Failed allocating memory", sn->id);
                goto fail;
@@ -270,19 +272,9 @@ static void *fr_schedule_network_thread(void *arg)
         */
        fr_network(sn->rc);
 
-       /*
-        *      Talloc ordering issues. We want to be independent of
-        *      how talloc walks it's children, and ensure that some
-        *      things are freed in a specific order.
-        */
-       fr_network_destroy(sn->rc);
-       sn->rc = NULL;
-
        status = FR_CHILD_EXITED;
 
 fail:
-       if (ctx) talloc_free(ctx);
-
        sn->status = status;
 
        fr_log(sc->log, L_INFO, "Network exiting");
@@ -416,6 +408,7 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el,
        SEM_WAIT_INTR(&sc->semaphore);
        if (sc->sn->status != FR_CHILD_RUNNING) {
        fail:
+               if (sc->sn->ctx) TALLOC_FREE(sc->sn->ctx);
                TALLOC_FREE(sc->sn);
                fr_schedule_destroy(sc);
                return NULL;
@@ -432,7 +425,7 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el,
                /*
                 *      Create a worker "glue" structure
                 */
-               sw = talloc(NULL, fr_schedule_worker_t);
+               sw = talloc(sc, fr_schedule_worker_t);
                if (!sw) {
                        fr_log(sc->log, L_ERR, "Worker %d - Failed allocating memory", i);
                        break;
@@ -446,7 +439,6 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el,
                rcode = pthread_create(&sw->pthread_id, &attr, fr_schedule_worker_thread, sw);
                if (rcode != 0) {
                        fr_log(sc->log, L_ERR, "Failed creating worker %d: %s\n", i, fr_syserror(errno));
-                       talloc_free(sw);
                        break;
                }
 
@@ -478,7 +470,6 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el,
                if (sw->status != FR_CHILD_RUNNING) {
                        sc->num_workers--;
                        fr_dlist_remove(entry);
-                       talloc_free(sw);
                        continue;
                }
        }
@@ -507,13 +498,17 @@ fr_schedule_t *fr_schedule_create(TALLOC_CTX *ctx, fr_event_list_t *el,
  */
 int fr_schedule_destroy(fr_schedule_t *sc)
 {
+#if 0
        int i;
        fr_schedule_worker_t *sw;
+#endif
 
        sc->running = false;
 
 #ifdef HAVE_PTHREAD_H
+#if 0
        fr_dlist_t      *entry, *next;
+#endif
 
        /*
         *      Single threaded mode: kill the only network / worker we have.
@@ -530,6 +525,19 @@ int fr_schedule_destroy(fr_schedule_t *sc)
 
        rad_assert(sc->num_workers > 0);
 
+       /*
+        *      If the network thread is running, tell it to exit, and
+        *      wait for it to do so.  Once it's exited, we know that
+        *      this thread can use the network channels to tell the
+        *      workers that the network side is going away.
+        */
+       if (sc->sn->status == FR_CHILD_RUNNING) {
+               fr_network_exit(sc->sn->rc);
+               SEM_WAIT_INTR(&sc->semaphore);
+               fr_network_destroy(sc->sn->rc);
+       }
+
+#if 0
        /*
         *      Signal all of the workers to exit.
         */
@@ -563,17 +571,14 @@ int fr_schedule_destroy(fr_schedule_t *sc)
                sw = fr_ptr_to_type(fr_schedule_worker_t, entry, entry);
                sc->num_workers--;
                fr_dlist_remove(entry);
-               talloc_free(sw);
+               fr_worker_destroy(sw->worker);
+//             talloc_free(sw->ctx);
        }
 
-       /*
-        *      If the network thread is running, tell it to exit.
-        */
-       if (sc->sn->status == FR_CHILD_RUNNING) {
-               fr_network_exit(sc->sn->rc);
-               SEM_WAIT_INTR(&sc->semaphore);
-       }
+       TALLOC_FREE(sc->sn->ctx);
 
+#endif
+       
        sem_destroy(&sc->semaphore);
 #endif /* HAVE_PTHREAD_H */
 

diff --git a/src/lib/io/worker.c b/src/lib/io/worker.c
index d895fa9710151e6bf0b376421bdaee0481858fef..12cf8a1cda844a75b28b390e290e15c5e1024fb8 100644 (file)
--- a/src/lib/io/worker.c
+++ b/src/lib/io/worker.c
@@ -1464,7 +1464,7 @@ void fr_worker(fr_worker_t *worker)
                num_events = fr_event_corral(worker->el, wait_for_event);
                DEBUG3("\t%sGot num_events %d", worker->name, num_events);
                if (num_events < 0) {
-                       if (worker->exiting) break; /* don't complain if we're exiting */
+                       if (worker->exiting) return; /* don't complain if we're exiting */
 
                        ERROR("Failed corraling events: %s", fr_strerror());
                        break;