From: Leonard Göhrs <l.goehrs@pengutronix.de>
Date: Fri, 24 Mar 2023 09:49:24 +0000 (+0100)
Subject: src/write_http.c: use reference counting to decide when to free user_data
X-Git-Tag: 6.0.0-rc0~38^2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88f388e7d2d61c3ed10a8f84a1ef81ec6353d144;p=thirdparty%2Fcollectd.git

src/write_http.c: use reference counting to decide when to free user_data

The teardown code for the wh_callback_t struct previously relied on
the order in which the different callback functions are de-initialized
to be known and to never change.

This is prone to failure and is indeed currently broken, leading to a
segmentation fault on collectd exit.

Fix this by counting the active references to the user data and freeing
it once it reaches zero.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
---

diff --git a/src/write_http.c b/src/write_http.c
index e31877c46..0a4c39247 100644
--- a/src/write_http.c
+++ b/src/write_http.c
@@ -92,6 +92,8 @@ struct wh_callback_s {
   char *metrics_prefix;
 
   char *unix_socket_path;
+
+  size_t reference_count;
 };
 typedef struct wh_callback_s wh_callback_t;
 
@@ -310,6 +312,17 @@ static void wh_callback_free(void *data) {
 
   wh_callback_t *cb = data;
 
+  /* cb is used as user_data in:
+   * - plugin_register_write
+   * - plugin_register_flush
+   * - plugin_register_notification
+   * We can not rely on them being torn down in a known order.
+   * Only actually free the structure when all references are dropped. */
+  cb->reference_count--;
+
+  if (cb->reference_count)
+    return;
+
   wh_flush(/* timeout = */ 0, NULL, &(user_data_t){.data = cb});
 
   if (cb->curl != NULL) {
@@ -544,6 +557,7 @@ static int wh_config_node(oconfig_item_t *ci) {
   cb->metrics_prefix = strdup(WRITE_HTTP_DEFAULT_PREFIX);
   cb->curl_stats = NULL;
   cb->unix_socket_path = NULL;
+  cb->reference_count = 1;
 
   if (cb->metrics_prefix == NULL) {
     ERROR("write_http plugin: strdup failed.");
@@ -700,17 +714,23 @@ static int wh_config_node(oconfig_item_t *ci) {
     ctx.flush_interval = plugin_get_interval();
     plugin_set_ctx(ctx);
 
+    cb->reference_count++;
     plugin_register_write(callback_name, wh_write, &user_data);
-    user_data.free_func = NULL;
 
+    cb->reference_count++;
     plugin_register_flush(callback_name, wh_flush, &user_data);
   }
 
   if (cb->send_notifications) {
+    cb->reference_count++;
     plugin_register_notification(callback_name, wh_notify, &user_data);
-    user_data.free_func = NULL;
   }
 
+  /* Drop the reference used in wh_config_node().
+   * `cb` will actually be freed once all references held in the callback
+   * handlers are dropped as well. */
+  wh_callback_free(cb);
+
   return 0;
 } /* int wh_config_node */