diff --git a/lib/persist-state.c b/lib/persist-state.c index e2002a00b2..89dda40dc8 100644 --- a/lib/persist-state.c +++ b/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) { diff --git a/lib/persist-state.h b/lib/persist-state.h index 6d92ea6f73..8fb43daf83 100644 --- a/lib/persist-state.h +++ b/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); diff --git a/modules/affile/file-reader.c b/modules/affile/file-reader.c index 78e2e0ef6d..4c6af0ae29 100644 --- a/modules/affile/file-reader.c +++ b/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); diff --git a/modules/affile/tests/test_wildcard_file_reader.c b/modules/affile/tests/test_wildcard_file_reader.c index bfafc9be1e..8ba6513582 100644 --- a/modules/affile/tests/test_wildcard_file_reader.c +++ b/modules/affile/tests/test_wildcard_file_reader.c @@ -31,6 +31,7 @@ #include #include #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); } diff --git a/modules/affile/wildcard-file-reader.c b/modules/affile/wildcard-file-reader.c index 7534c38d5e..9442d53180 100644 --- a/modules/affile/wildcard-file-reader.c +++ b/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; diff --git a/modules/affile/wildcard-source.c b/modules/affile/wildcard-source.c index 2015a9c284..b00a11bd84 100644 --- a/modules/affile/wildcard-source.c +++ b/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, diff --git a/news/bugfix-291.md b/news/bugfix-291.md new file mode 100644 index 0000000000..ac55abd5d8 --- /dev/null +++ b/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.