From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 6 Aug 2025 14:59:08 +0000 (-0400)
Subject: clean up shutdown and destructor
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d3cbe98f6994e36861aa4aee681e7bfc7d376e3e;p=thirdparty%2Ffreeradius-server.git

clean up shutdown and destructor

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.
---

diff --git a/src/lib/bio/base.c b/src/lib/bio/base.c
index 6898de53fc..05b217ba4e 100644
--- a/src/lib/bio/base.c
+++ b/src/lib/bio/base.c
@@ -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;
 }
diff --git a/src/lib/bio/base.h b/src/lib/bio/base.h
index 68ac002bb1..1d08a38899 100644
--- a/src/lib/bio/base.h
+++ b/src/lib/bio/base.h
@@ -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
 
diff --git a/src/lib/bio/bio_priv.h b/src/lib/bio/bio_priv.h
index 55195b4b9d..4c42a3216c 100644
--- a/src/lib/bio/bio_priv.h
+++ b/src/lib/bio/bio_priv.h
@@ -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;
 
diff --git a/src/lib/bio/fd.c b/src/lib/bio/fd.c
index a343a08f99..813f152df4 100644
--- a/src/lib/bio/fd.c
+++ b/src/lib/bio/fd.c
@@ -88,17 +88,14 @@
 		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)
diff --git a/src/lib/bio/fd_open.c b/src/lib/bio/fd_open.c
index 07ec5711c9..dc38a50d66 100644
--- a/src/lib/bio/fd_open.c
+++ b/src/lib/bio/fd_open.c
@@ -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.
diff --git a/src/lib/bio/fd_priv.h b/src/lib/bio/fd_priv.h
index 98fb7b914e..69bab4689c 100644
--- a/src/lib/bio/fd_priv.h
+++ b/src/lib/bio/fd_priv.h
@@ -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;
 
diff --git a/src/lib/bio/packet.c b/src/lib/bio/packet.c
index b035fa5b09..b0ece95e62 100644
--- a/src/lib/bio/packet.c
+++ b/src/lib/bio/packet.c
@@ -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)
diff --git a/src/lib/bio/packet.h b/src/lib/bio/packet.h
index 68fb998aa2..af51164965 100644
--- a/src/lib/bio/packet.h
+++ b/src/lib/bio/packet.h
@@ -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;
 
diff --git a/src/lib/bio/pipe.c b/src/lib/bio/pipe.c
index a4cf06eec5..ccfd7f0422 100644
--- a/src/lib/bio/pipe.c
+++ b/src/lib/bio/pipe.c
@@ -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.
diff --git a/src/lib/bio/queue.c b/src/lib/bio/queue.c
index 6c3f2996bb..f6aa6c3cce 100644
--- a/src/lib/bio/queue.c
+++ b/src/lib/bio/queue.c
@@ -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;
 }
 
 
diff --git a/src/lib/bio/retry.c b/src/lib/bio/retry.c
index f4a03bc9c2..1e85472538 100644
--- a/src/lib/bio/retry.c
+++ b/src/lib/bio/retry.c
@@ -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