]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
clean up shutdown and destructor
authorAlan T. DeKok <aland@freeradius.org>
Wed, 6 Aug 2025 14:59:08 +0000 (10:59 -0400)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 6 Aug 2025 17:41:08 +0000 (13:41 -0400)
some shutdowns can fail, so the function needs to return an rcode.

the destructors should just call the shutdown, so the caller can
just talloc_free() things, and have it all work properly.

the shutdown doesn't need to reset the destructors, as the main
fr_bio_shutdown() will do that.

src/lib/bio/base.c
src/lib/bio/base.h
src/lib/bio/bio_priv.h
src/lib/bio/fd.c
src/lib/bio/fd_open.c
src/lib/bio/fd_priv.h
src/lib/bio/packet.c
src/lib/bio/packet.h
src/lib/bio/pipe.c
src/lib/bio/queue.c
src/lib/bio/retry.c

index 6898de53fc3cfe848d74a44cdafb2087ba0f42d8..05b217ba4ef700dabe84161785c47478a19a85e9 100644 (file)
@@ -150,6 +150,7 @@ static ssize_t fr_bio_shutdown_write(UNUSED fr_bio_t *bio, UNUSED void *packet_c
  */
 int fr_bio_shutdown(fr_bio_t *bio)
 {
+       int rcode;
        fr_bio_t *this, *first;
        fr_bio_common_t *my;
 
@@ -174,7 +175,8 @@ int fr_bio_shutdown(fr_bio_t *bio)
                my = (fr_bio_common_t *) this;
 
                if (my->priv_cb.shutdown) {
-                       my->priv_cb.shutdown(&my->bio);
+                       rcode = my->priv_cb.shutdown(&my->bio);
+                       if (rcode < 0) return rcode;
                        my->priv_cb.shutdown = NULL;
                }
 
@@ -188,8 +190,11 @@ int fr_bio_shutdown(fr_bio_t *bio)
         */
        my = (fr_bio_common_t *) first;
 
-       if (my->cb.shutdown) my->cb.shutdown(first);
-       my->cb.shutdown = NULL;
+       if (my->cb.shutdown) {
+               rcode = my->cb.shutdown(first);
+               if (rcode < 0) return rcode;
+               my->cb.shutdown = NULL;
+       }
 
        return 0;
 }
index 68ac002bb10441641d9df497afa747d18a02bbe0..1d08a388992e2d38b11dd334793799ed769398af 100644 (file)
@@ -87,7 +87,7 @@ typedef void (*fr_bio_callback_t)(fr_bio_t *bio); /* connected / shutdown callba
 
 typedef struct {
        fr_bio_callback_t       connected;              //!< called when the BIO is ready to be used
-       fr_bio_callback_t       shutdown;               //!< called when the BIO is being shut down
+       fr_bio_io_t             shutdown;               //!< called when the BIO is being shut down
        fr_bio_callback_t       eof;                    //!< called when the BIO is at EOF
        fr_bio_callback_t       failed;                 //!< called when the BIO fails
 
index 55195b4b9de3fa5aebb18f7b86820b4b2f29fdb1..4c42a3216c3b4517e62258c4c788cf633d1c6089 100644 (file)
@@ -29,13 +29,11 @@ RCSIDH(lib_bio_bio_priv_h, "$Id$")
 #define _BIO_PRIVATE 1
 #include <freeradius-devel/bio/base.h>
 
-typedef int (*fr_bio_shutdown_t)(fr_bio_t *bio);
-
 typedef struct fr_bio_common_s fr_bio_common_t;
 
 typedef struct {
        fr_bio_io_t             connected;
-       fr_bio_callback_t       shutdown;
+       fr_bio_io_t             shutdown;
        fr_bio_io_t             eof;
        fr_bio_callback_t       failed;
 
index a343a08f99e735166aee23d958dd7bedb46e4301..813f152df4a7bafd07976d187849fdba1cc92aa6 100644 (file)
                addr->socket.inet.ifindex = my->info.socket.inet.ifindex; \
        } while (0)
 
-/*
- *     Close the descriptor and free the bio.
+/**  Orderly shutdown.
+ *
  */
-static int fr_bio_fd_destructor(fr_bio_fd_t *my)
+static int fr_bio_fd_shutdown(fr_bio_t *bio)
 {
-       /*
-        *      The upstream bio must have unlinked it from the chain before calling talloc_free() on this
-        *      bio.
-        */
-       fr_assert(!fr_bio_prev(&my->bio));
-       fr_assert(!fr_bio_next(&my->bio));
+       fr_bio_fd_t *my = talloc_get_type_abort(bio, fr_bio_fd_t);
+
+       if (my->info.state == FR_BIO_FD_STATE_CLOSED) return 0;
 
        FR_TIMER_DELETE(&my->connect.ev);
        if (my->connect.el) {
@@ -109,15 +106,9 @@ static int fr_bio_fd_destructor(fr_bio_fd_t *my)
        return fr_bio_fd_close(&my->bio);
 }
 
-/**  Orderly shutdown.
- *
- */
-static void fr_bio_fd_shutdown(fr_bio_t *bio)
+static int fr_bio_fd_destructor(fr_bio_fd_t *my)
 {
-       fr_bio_fd_t *my = talloc_get_type_abort(bio, fr_bio_fd_t);
-
-       (void) fr_bio_fd_destructor(my);
-       talloc_set_destructor(my, NULL);
+       return fr_bio_fd_shutdown((fr_bio_t *) my);
 }
 
 static int fr_bio_fd_eof(fr_bio_t *bio)
index 07ec5711c90f4b15132845b6ccd301f6404ea694..dc38a50d66cbbc9486afe1237c3bc9ae455f1809 100644 (file)
@@ -486,7 +486,7 @@ static int fr_bio_fd_socket_unix_mkdir(int *dirfd, char const **filename, fr_bio
        return 0;
 }
 
-static void fr_bio_fd_unix_shutdown(fr_bio_t *bio)
+static int fr_bio_fd_unix_shutdown(fr_bio_t *bio)
 {
        fr_bio_fd_t *my = talloc_get_type_abort(bio, fr_bio_fd_t);
 
@@ -503,9 +503,16 @@ static void fr_bio_fd_unix_shutdown(fr_bio_t *bio)
        /*
         *      Run the user shutdown before we run ours.
         */
-       if (my->user_shutdown) my->user_shutdown(bio);
+       if (my->user_shutdown) {
+               int rcode;
 
-       (void) unlink(my->info.socket.unix.path);
+               rcode = my->user_shutdown(bio);
+               if (rcode < 0) return rcode;
+       }
+
+       if (unlink(my->info.socket.unix.path) < 0) return fr_bio_error(GENERIC);
+
+       return 0;
 }
 
 /** Bind to a Unix domain socket.
index 98fb7b914eff475442951711257c5139fb2bacc9..69bab4689ce9aa7311f5cde3173090b9877df1ac 100644 (file)
@@ -34,7 +34,7 @@ RCSIDH(lib_bio_fd_privh, "$Id$")
  */
 typedef struct fr_bio_fd_s {
        FR_BIO_COMMON;
-       fr_bio_callback_t  user_shutdown;       //!< user shutdown
+       fr_bio_io_t  user_shutdown;                             //!< user shutdown
 
        fr_bio_fd_info_t  info;
 
index b035fa5b094a2a8c1d72a56ec488115edc554640..b0ece95e62555d432f58411e8987ce2974473b19 100644 (file)
@@ -149,12 +149,13 @@ void fr_bio_packet_connected(fr_bio_t *bio)
        my->cb.connected(my);
 }
 
-static void fr_bio_packet_shutdown(fr_bio_t *bio)
+static int fr_bio_packet_shutdown(fr_bio_t *bio)
 {
        fr_bio_packet_t *my = bio->uctx;
 
-       if (my->cb.shutdown) my->cb.shutdown(my);
-       my->cb.shutdown = NULL;
+       if (!my->cb.shutdown) return 0;
+
+       return my->cb.shutdown(my);
 }
 
 static void fr_bio_packet_eof(fr_bio_t *bio)
index 68fb998aa292241dc2679ad18de57fb2d1163035..af51164965229120116d4fad82dd9b55cf06c3b0 100644 (file)
@@ -69,7 +69,7 @@ typedef void (*fr_bio_packet_callback_t)(fr_bio_packet_t *bio);
 
 typedef struct {
        fr_bio_packet_callback_t        connected;
-       fr_bio_packet_callback_t        shutdown;
+       fr_bio_packet_io_t              shutdown;
        fr_bio_packet_callback_t        eof;
        fr_bio_packet_callback_t        failed;
 
index a4cf06eec50227ddf42a428f14c9332cb4ed6b37..ccfd7f042265773d903a1befc334d695e1ddb618 100644 (file)
@@ -124,15 +124,18 @@ static ssize_t fr_bio_pipe_write(fr_bio_t *bio, void *packet_ctx, void const *bu
 /** Shutdown callback.
  *
  */
-static void fr_bio_pipe_shutdown(fr_bio_t *bio)
+static int fr_bio_pipe_shutdown(fr_bio_t *bio)
 {
+       int rcode;
        fr_bio_pipe_t *my = talloc_get_type_abort(bio, fr_bio_pipe_t);  
 
        fr_assert(my->next != NULL);
 
        pthread_mutex_lock(&my->mutex);
-       fr_bio_shutdown(my->next);
+       rcode = fr_bio_shutdown(my->next);
        pthread_mutex_unlock(&my->mutex);
+
+       return rcode;
 }
 
 /** Set EOF.
index 6c3f2996bb9624e9043d8e877db66e8179b53787..f6aa6c3cce15ebc1427d05336fae72040b7b7e97 100644 (file)
@@ -334,11 +334,13 @@ static ssize_t fr_bio_queue_read(fr_bio_t *bio, void *packet_ctx, void *buffer,
  *
  *  Cancel / close has to be called before re-init.
  */
-static void fr_bio_queue_shutdown(fr_bio_t *bio)
+static int fr_bio_queue_shutdown(fr_bio_t *bio)
 {
        fr_bio_queue_t *my = talloc_get_type_abort(bio, fr_bio_queue_t);
 
        fr_bio_queue_list_cancel(my);
+
+       return 0;
 }
 
 
index f4a03bc9c2d845aa09052afd4e3c693814440dfb..1e8547253847a3dfe1463d191f7de663bd403996 100644 (file)
@@ -862,12 +862,12 @@ const fr_retry_t *fr_bio_retry_entry_info(UNUSED fr_bio_t *bio, fr_bio_retry_ent
        return &item->retry;
 }
 
-
-/**  Cancel all outstanding packets.
+/**  Orderly shutdown.
  *
  */
-static int fr_bio_retry_destructor(fr_bio_retry_t *my)
+static int fr_bio_retry_shutdown(fr_bio_t *bio)
 {
+       fr_bio_retry_t *my = talloc_get_type_abort(bio, fr_bio_retry_t);
        fr_bio_retry_entry_t *item;
 
        fr_timer_list_disarm(my->next_tl);
@@ -885,15 +885,9 @@ static int fr_bio_retry_destructor(fr_bio_retry_t *my)
        return 0;
 }
 
-/**  Orderly shutdown.
- *
- */
-static void fr_bio_retry_shutdown(fr_bio_t *bio)
+static int fr_bio_retry_destructor(fr_bio_retry_t *my)
 {
-       fr_bio_retry_t *my = talloc_get_type_abort(bio, fr_bio_retry_t);
-
-       (void) fr_bio_retry_destructor(my);
-       talloc_set_destructor(my, NULL);
+       return fr_bio_retry_shutdown((fr_bio_t *) my);
 }
 
 /**  Allocate a #fr_bio_retry_t