wildcard-file: fix persist-name uniqueness with user-supplied name and collision with regular file sources

lib/persist-state.c

@@ -359,6 +359,28 @@ _persist_state_copy_entry(PersistState *self, const PersistEntryHandle from, Per
   persist_state_unmap_entry(self, to);
 }
 
+gboolean
+persist_state_copy_entry(PersistState *self, const gchar *old_key, const gchar *new_key)
+{
+  gsize size;
+  guint8 version;
+  PersistEntryHandle old_handle = persist_state_lookup_entry(self, old_key, &size, &version);
+  if (!old_handle)
+    return FALSE;
+
+  PersistEntryHandle new_handle = persist_state_alloc_entry(self, new_key, size);
+  if (!new_handle)
+    return FALSE;
+
+  _persist_state_copy_entry(self, old_handle, new_handle, size);
+
+  msg_debug("Persistent entry copied",
+            evt_tag_str("from", old_key),
+            evt_tag_str("to", new_key));
+
+  return TRUE;
+}
+
 gboolean
 persist_state_move_entry(PersistState *self, const gchar *old_key, const gchar *new_key)
 {

lib/persist-state.h

@@ -98,6 +98,7 @@ gboolean persist_state_remove_entry(PersistState *self, const gchar *persist_nam
 
 gchar *persist_state_lookup_string(PersistState *self, const gchar *key, gsize *length, guint8 *version);
 gboolean persist_state_rename_entry(PersistState *self, const gchar *old_key, const gchar *new_key);
+gboolean persist_state_copy_entry(PersistState *self, const gchar *old_key, const gchar *new_key);
 gboolean persist_state_move_entry(PersistState *self, const gchar *old_key, const gchar *new_key);
 void persist_state_alloc_string(PersistState *self, const gchar *persist_name, const gchar *value, gssize len);
 

modules/affile/file-reader.c

@@ -69,7 +69,7 @@ _recover_state(LogPipe *s, GlobalConfig *cfg, LogProtoServer *proto)
   if (!self->options->restore_state)
     return;
 
-  if (!log_proto_server_restart_with_state(proto, cfg->state, _format_persist_name(s)))
+  if (!log_proto_server_restart_with_state(proto, cfg->state, log_pipe_get_persist_name(s)))
     {
       msg_error("Error converting persistent state from on-disk format, losing file position information",
                 evt_tag_str("filename", self->filename->str));
@@ -325,7 +325,7 @@ void
 file_reader_remove_persist_state(FileReader *self)
 {
   GlobalConfig *cfg = log_pipe_get_config(&self->super);
-  const gchar *old_persist_name = _format_persist_name(&self->super);
+  const gchar *old_persist_name = log_pipe_get_persist_name(&self->super);
   gchar *new_persist_name = g_strdup_printf("%s_REMOVED", old_persist_name);
   /* This is required to clean the persist entry from file during restart */
   persist_state_remove_entry(cfg->state, old_persist_name);

modules/affile/tests/test_wildcard_file_reader.c

@@ -31,6 +31,7 @@
 #include <glib/gstdio.h>
 #include <unistd.h>
 #include "poll-file-changes.h"
+#include "cfg.h"
 
 #define TEST_FILE_NAME "TEST_FILE"
 
@@ -110,7 +111,7 @@ _init(void)
 {
   app_startup();
   test_event = test_deleted_file_state_event_new();
-  reader = (WildcardFileReader *)wildcard_file_reader_new(TEST_FILE_NAME, NULL, NULL, NULL, NULL);
+  reader = (WildcardFileReader *)wildcard_file_reader_new(TEST_FILE_NAME, NULL, NULL, NULL, cfg_new_snippet());
   wildcard_file_reader_on_deleted_file_eof(reader, _eof, test_event);
   cr_assert_eq(log_pipe_init(&reader->super.super), TRUE);
 }

modules/affile/wildcard-file-reader.c

@@ -26,12 +26,66 @@
 #include "mainloop.h"
 #include "poll-file-changes.h"
 
+static inline const gchar *
+_format_persist_name(const LogPipe *s)
+{
+  const FileReader *self = (const FileReader *)s;
+  static gchar persist_name[1024];
+
+  if (self->owner->super.super.persist_name)
+    {
+      g_snprintf(persist_name, sizeof(persist_name), "wildcard_file_sd.%s.curpos(%s)",
+                 self->owner->super.super.persist_name, self->filename->str);
+    }
+  else
+    g_snprintf(persist_name, sizeof(persist_name), "wildcard_file_sd_curpos(%s)", self->filename->str);
+
+  return persist_name;
+}
+
+static inline const gchar *
+_format_legacy_persist_name(const LogPipe *s)
+{
+  const FileReader *self = (const FileReader *)s;
+  static gchar persist_name[1024];
+
+  if (self->owner->super.super.persist_name)
+    g_snprintf(persist_name, sizeof(persist_name), "affile_sd.%s.curpos", self->owner->super.super.persist_name);
+  else
+    g_snprintf(persist_name, sizeof(persist_name), "affile_sd_curpos(%s)", self->filename->str);
+
+  return persist_name;
+}
+
+static gboolean
+_update_legacy_persist_name(WildcardFileReader *self)
+{
+  GlobalConfig *cfg = log_pipe_get_config(&self->super.super);
+
+  if (!cfg->state)
+    return TRUE;
+
+  const gchar *current_persist_name = _format_persist_name(&self->super.super);
+  const gchar *legacy_persist_name = _format_legacy_persist_name(&self->super.super);
+
+  if (persist_state_entry_exists(cfg->state, current_persist_name))
+    return TRUE;
+
+  if (!persist_state_entry_exists(cfg->state, legacy_persist_name))
+    return TRUE;
+
+  return persist_state_copy_entry(cfg->state, legacy_persist_name, current_persist_name);
+}
+
 static gboolean
 _init(LogPipe *s)
 {
   WildcardFileReader *self = (WildcardFileReader *)s;
   self->file_state.deleted = FALSE;
   self->file_state.last_eof = FALSE;
+
+  _update_legacy_persist_name(self);
+
   return file_reader_init_method(s);
 }
 
@@ -148,6 +202,7 @@ wildcard_file_reader_new(const gchar *filename, FileReaderOptions *options, File
   self->super.super.init = _init;
   self->super.super.notify = _notify;
   self->super.super.deinit = _deinit;
+  self->super.super.generate_persist_name = _format_persist_name;
   IV_TASK_INIT(&self->file_state_event_handler);
   self->file_state_event_handler.cookie = self;
   self->file_state_event_handler.handler = _handle_file_state_event;

modules/affile/wildcard-source.c

@@ -419,6 +419,21 @@ wildcard_sd_set_max_files(LogDriver *s, guint32 max_files)
   self->max_files = max_files;
 }
 
+/* to validate init-time uniqueness */
+static inline const gchar *
+_format_persist_name(const LogPipe *s)
+{
+  const WildcardSourceDriver *self = (const WildcardSourceDriver *)s;
+  static gchar persist_name[1024];
+
+  if (self->super.super.super.persist_name)
+    g_snprintf(persist_name, sizeof(persist_name), "wildcard_file_sd.%s", self->super.super.super.persist_name);
+  else
+    g_snprintf(persist_name, sizeof(persist_name), "wildcard_file_sd(%s,%s)", self->base_dir, self->filename_pattern);
+
+  return persist_name;
+}
+
 static void
 _free(LogPipe *s)
 {
@@ -445,6 +460,7 @@ wildcard_sd_new(GlobalConfig *cfg)
   self->super.super.super.free_fn = _free;
   self->super.super.super.init = _init;
   self->super.super.super.deinit = _deinit;
+  self->super.super.super.generate_persist_name = _format_persist_name;
 
   self->file_readers = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)log_pipe_unref);
   self->directory_monitors = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,

news/bugfix-291.md

@@ -0,0 +1,4 @@
+`file()`, `wildcard-file()`: fix crash and persist name collision issues
+
+If multiple `wildcard-file()` sources or a `wildcard-file()` and a `file()` source were reading the same input file,
+it could result in log loss, log duplication, and various crashes.