zebra: fix race condition in i/o pthread shutdown

I mistakenly used an external mechanism to cause a pthread to shut
itself down instead of using the one built into frr_pthread.[ch]. This
created a race condition whereby a pthread could schedule work onto a
dead pthread and cause it to reanimate.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/zebra/zserv.c b/zebra/zserv.c
index a264add420757a9480ed6fe883fc59b4ad72bd20..a099cfc057ddf941e9f694ee49d0aebd44b32deb 100644 (file)
--- a/zebra/zserv.c
+++ b/zebra/zserv.c
@@ -167,7 +167,8 @@ static void zserv_log_message(const char *errmsg, struct stream *msg,
  */
 static void zserv_client_close(struct zserv *client)
 {
-       atomic_store_explicit(&client->dead, true, memory_order_seq_cst);
+       atomic_store_explicit(&client->pthread->running, false,
+                             memory_order_seq_cst);
        THREAD_OFF(client->t_read);
        THREAD_OFF(client->t_write);
        zserv_event(client, ZSERV_HANDLE_CLOSE);
@@ -200,9 +201,6 @@ static int zserv_write(struct thread *thread)
        uint32_t wcmd;
        struct stream_fifo *cache;
 
-       if (atomic_load_explicit(&client->dead, memory_order_seq_cst))
-               return 0;
-
        /* If we have any data pending, try to flush it first */
        switch (buffer_flush_all(client->wb, client->sock)) {
        case BUFFER_ERROR:
@@ -221,7 +219,7 @@ static int zserv_write(struct thread *thread)
 
        pthread_mutex_lock(&client->obuf_mtx);
        {
-               while (client->obuf_fifo->head)
+               while (stream_fifo_head(client->obuf_fifo))
                        stream_fifo_push(cache,
                                         stream_fifo_pop(client->obuf_fifo));
        }
@@ -251,7 +249,6 @@ static int zserv_write(struct thread *thread)
                                      memory_order_relaxed);
                zserv_client_event(client, ZSERV_CLIENT_WRITE);
                return 0;
-               break;
        case BUFFER_EMPTY:
                break;
        }
@@ -304,9 +301,6 @@ static int zserv_read(struct thread *thread)
        uint32_t p2p;
        struct zmsghdr hdr;
 
-       if (atomic_load_explicit(&client->dead, memory_order_seq_cst))
-               return 0;
-
        p2p_orig = atomic_load_explicit(&zebrad.packets_to_process,
                                        memory_order_relaxed);
        cache = stream_fifo_new();
@@ -449,9 +443,6 @@ zread_fail:
 static void zserv_client_event(struct zserv *client,
                               enum zserv_client_event event)
 {
-       if (atomic_load_explicit(&client->dead, memory_order_seq_cst))
-               return;
-
        switch (event) {
        case ZSERV_CLIENT_READ:
                thread_add_read(client->pthread->master, zserv_read, client,

diff --git a/zebra/zserv.h b/zebra/zserv.h
index 2a681552d16f583ff54e41be8cdf583babee6907..78cc200fa8c4576aff24a4669ebba1e1143e44ac 100644 (file)
--- a/zebra/zserv.h
+++ b/zebra/zserv.h
@@ -53,9 +53,6 @@ struct zserv {
        /* Client pthread */
        struct frr_pthread *pthread;
 
-       /* Whether the thread is waiting to be killed */
-       _Atomic bool dead;
-
        /* Client file descriptor. */
        int sock;