From f824cde45fb5dfb491e28918b1de0c5564520950 Mon Sep 17 00:00:00 2001 From: Jabber Developer Date: Wed, 2 Jul 2025 12:14:35 +0200 Subject: [PATCH 1/4] fix(mam, log): improve datetime handling and memory management in MAM and log fetching - Introduce static helper `_truncate_datetime_suffix()` to safely trim datetime strings, removing unwanted suffixes like timezone offsets - Replace manual string management with auto_gchar and g_strdup for safer, clearer ownership and to prevent leaks - Add safety checks and logging warnings for unexpected datetime string lengths or null pointers - Refactor _mam_rsm_id_handler to use the helper function and updated string handling - Change log_database_get_previous_chat parameters for consistent ownership semantics, avoiding double frees and mem leaks - Overall improve stability and prevent memory leaks during log database queries --- src/database.c | 4 ++-- src/database.h | 2 +- src/ui/chatwin.c | 9 ++++++--- src/ui/ui.h | 2 +- src/ui/window.c | 8 +++----- src/xmpp/iq.c | 28 ++++++++++++++++++++++------ src/xmpp/presence.c | 2 +- 7 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/database.c b/src/database.c index 5ceb6357..6abdb6b8 100644 --- a/src/database.c +++ b/src/database.c @@ -320,7 +320,7 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las // null the current time is used. from_start gets first few messages if true // otherwise the last ones. Flip flips the order of the results db_history_result_t -log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip, GSList** result) +log_database_get_previous_chat(const gchar* const contact_barejid, const gchar* start_time, const gchar* end_time, gboolean from_start, gboolean flip, GSList** result) { if (!g_chatlog_database) { log_warning("log_database_get_previous_chat() called but db is not initialized"); @@ -337,7 +337,7 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const char* s gchar* sort1 = from_start ? "ASC" : "DESC"; gchar* sort2 = !flip ? "ASC" : "DESC"; GDateTime* now = g_date_time_new_now_local(); - auto_gchar gchar* end_date_fmt = end_time ? end_time : g_date_time_format_iso8601(now); + auto_gchar gchar* end_date_fmt = end_time ? g_strdup(end_time) : g_date_time_format_iso8601(now); auto_sqlite gchar* query = sqlite3_mprintf("SELECT * FROM (" "SELECT COALESCE(B.`message`, A.`message`) AS message, " "A.`timestamp`, A.`from_jid`, A.`from_resource`, A.`to_jid`, A.`to_resource`, A.`type`, A.`encryption`, A.`stanza_id` FROM `ChatLogs` AS A " diff --git a/src/database.h b/src/database.h index 4b84e722..e6c28359 100644 --- a/src/database.h +++ b/src/database.h @@ -53,7 +53,7 @@ void log_database_add_incoming(ProfMessage* message); void log_database_add_outgoing_chat(const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc); void log_database_add_outgoing_muc(const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc); void log_database_add_outgoing_muc_pm(const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc); -db_history_result_t log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip, GSList** result); +db_history_result_t log_database_get_previous_chat(const gchar* const contact_barejid, const gchar* start_time, const gchar* end_time, gboolean from_start, gboolean flip, GSList** result); ProfMessage* log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_last); void log_database_close(void); diff --git a/src/ui/chatwin.c b/src/ui/chatwin.c index fc14f0a5..922b8523 100644 --- a/src/ui/chatwin.c +++ b/src/ui/chatwin.c @@ -40,6 +40,7 @@ #include #include +#include "glib.h" #include "xmpp/chat_session.h" #include "window_list.h" #include "xmpp/roster_list.h" @@ -585,10 +586,12 @@ _chatwin_history(ProfChatWin* chatwin, const char* const contact_barejid) // first entry's timestamp in the buffer is used. Flip true to prepend to buffer. // Timestamps should be in iso8601 db_history_result_t -chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip) +chatwin_db_history(ProfChatWin* chatwin, const gchar* start_time, const gchar* end_time, gboolean flip) { - if (!end_time) { - end_time = buffer_size(((ProfWin*)chatwin)->layout->buffer) == 0 ? NULL : g_date_time_format_iso8601(buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0)->time); + auto_gchar gchar* _end_time = NULL; + if (!end_time && buffer_size(((ProfWin*)chatwin)->layout->buffer) > 0) { + _end_time = g_date_time_format_iso8601(buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0)->time); + end_time = _end_time; } GSList* history = NULL; diff --git a/src/ui/ui.h b/src/ui/ui.h index 1eafd743..97b7c32e 100644 --- a/src/ui/ui.h +++ b/src/ui/ui.h @@ -146,7 +146,7 @@ void chatwin_set_incoming_char(ProfChatWin* chatwin, const char* const ch); void chatwin_unset_incoming_char(ProfChatWin* chatwin); void chatwin_set_outgoing_char(ProfChatWin* chatwin, const char* const ch); void chatwin_unset_outgoing_char(ProfChatWin* chatwin); -db_history_result_t chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip); +db_history_result_t chatwin_db_history(ProfChatWin* chatwin, const gchar* start_time, const gchar* end_time, gboolean flip); // MUC window ProfMucWin* mucwin_new(const char* const barejid); diff --git a/src/ui/window.c b/src/ui/window.c index 6489142a..9bce0f7b 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -711,19 +711,17 @@ win_page_down(ProfWin* window, int scroll_size) if ((*page_start > total_rows - page_space || (*page_start == page_space && *page_start >= total_rows)) && window->type == WIN_CHAT) { int bf_size = buffer_size(window->layout->buffer); if (bf_size > 0) { + GDateTime* now = g_date_time_new_now_local(); ProfBuffEntry* last_entry = buffer_get_entry(window->layout->buffer, bf_size - 1); auto_gchar gchar* start = g_date_time_format_iso8601(last_entry->time); - GDateTime* now = g_date_time_new_now_local(); - gchar* end_date = g_date_time_format_iso8601(now); - g_date_time_unref(now); + auto_gchar gchar* end_date = g_date_time_format_iso8601(now); if (*scroll_state != WIN_SCROLL_REACHED_BOTTOM && !chatwin_db_history((ProfChatWin*)window, start, end_date, FALSE)) { *scroll_state = WIN_SCROLL_REACHED_BOTTOM; - } else if (*scroll_state == WIN_SCROLL_REACHED_BOTTOM) { - g_free(end_date); } int offset = last_entry->y_end_pos - 1; *page_start = offset - page_space + scroll_size; + g_date_time_unref(now); } } diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index a8a485b2..b3a1bf98 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -2752,6 +2752,22 @@ iq_mam_request(ProfChatWin* win, GDateTime* enddate) return; } +// Trim the last 3 characters from the datetime string to remove unwanted suffix (timezone offset) +static void +_truncate_datetime_suffix(gchar* str) +{ + if (!str) { + log_warning("_truncate_datetime_suffix(): unexpected null datetime string"); + return; + } + size_t len = strlen(str); + if (len >= 3) { + str[len - 3] = '\0'; + } else { + log_warning("_truncate_datetime_suffix(): unexpected short datetime string: %s", str); + } +} + static int _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata) { @@ -2773,17 +2789,17 @@ _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata) buffer_remove_entry(window->layout->buffer, 0); - auto_char char* start_str = NULL; + auto_gchar gchar* start_str = NULL; if (data->start_datestr) { - start_str = strdup(data->start_datestr); + start_str = g_strdup(data->start_datestr); // Convert to iso8601 - start_str[strlen(start_str) - 3] = '\0'; + _truncate_datetime_suffix(start_str); } - char* end_str = NULL; + auto_gchar gchar* end_str = NULL; if (data->end_datestr) { - end_str = strdup(data->end_datestr); + end_str = g_strdup(data->end_datestr); // Convert to iso8601 - end_str[strlen(end_str) - 3] = '\0'; + _truncate_datetime_suffix(end_str); } if (is_complete || !data->fetch_next) { diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 577a2e82..56cbb379 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -644,7 +644,7 @@ _available_handler(xmpp_stanza_t* const stanza) const char* account_name = session_get_account_name(); int max_sessions = accounts_get_max_sessions(account_name); if (max_sessions > 0) { - const gchar* cur_resource = accounts_get_resource(account_name); + auto_gchar gchar* cur_resource = accounts_get_resource(account_name); int res_count = connection_count_available_resources(); if (res_count > max_sessions && g_strcmp0(cur_resource, resource->name)) { ProfWin* console = wins_get_console(); -- 2.49.1 From 6871a4d46fea2df7fd397a67499f72ddde639fc8 Mon Sep 17 00:00:00 2001 From: Jabber Developer Date: Wed, 2 Jul 2025 15:05:44 +0200 Subject: [PATCH 2/4] Fix memory leak in _handle_string_or_none_result The function did not decref the Python object on type error paths, resulting in a reference count leak. This caused memory to accumulate over time. Added Py_XDECREF on all early return paths to properly free the Python object reference and prevent leaks. --- src/plugins/python_plugins.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plugins/python_plugins.c b/src/plugins/python_plugins.c index 8cd721df..13278750 100644 --- a/src/plugins/python_plugins.c +++ b/src/plugins/python_plugins.c @@ -900,6 +900,8 @@ _python_type_error(ProfPlugin* plugin, char* hook, char* type) g_string_free(err_msg, TRUE); } +// Converts a Python string or None result to a C string and decreases the Python object's reference count. +// If the result is NULL, or not a string/unicode/bytes/None, logs an error and returns NULL. static char* _handle_string_or_none_result(ProfPlugin* plugin, PyObject* result, char* hook) { @@ -910,18 +912,16 @@ _handle_string_or_none_result(ProfPlugin* plugin, PyObject* result, char* hook) } #ifdef PY_IS_PYTHON3 if (result != Py_None && !PyUnicode_Check(result) && !PyBytes_Check(result)) { - allow_python_threads(); - _python_type_error(plugin, hook, "string, unicode or None"); - return NULL; - } #else if (result != Py_None && !PyUnicode_Check(result) && !PyString_Check(result)) { +#endif + Py_XDECREF(result); allow_python_threads(); _python_type_error(plugin, hook, "string, unicode or None"); return NULL; } -#endif char* result_str = python_str_or_unicode_to_string(result); + Py_XDECREF(result); allow_python_threads(); return result_str; } -- 2.49.1 From a5eab3cad5bfc41478fb4f6704ad2a27b5d11cf1 Mon Sep 17 00:00:00 2001 From: Jabber Developer Date: Wed, 2 Jul 2025 15:21:44 +0200 Subject: [PATCH 3/4] build: Upgrade valgrind suppression file Use newer version of Python's .supp file. --- prof.supp | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/prof.supp b/prof.supp index 36f9ef71..c2f7f59c 100644 --- a/prof.supp +++ b/prof.supp @@ -5,7 +5,7 @@ # # This file is a combination of our own rules and # * the glib2 suppressions file that usually can be found at /usr/share/glib-2.0/valgrind/glib.supp -# * python suppressions file from http://svn.python.org/projects/python/trunk/Misc/valgrind-python.supp +# * python suppressions file from https://github.com/python/cpython/blob/main/Misc/valgrind-python.supp # { @@ -1557,6 +1557,10 @@ ... fun:xdg_mime_init } + +# Python Valgrind .supp start +# taken from https://github.com/python/cpython/blob/main/Misc/valgrind-python.supp + # # This is a valgrind suppression file that should be used when using valgrind. # @@ -1654,6 +1658,49 @@ fun:COMMENT_THIS_LINE_TO_DISABLE_LEAK_WARNING } +# +# Leaks: dlopen() called without dlclose() +# + +{ + dlopen() called without dlclose() + Memcheck:Leak + fun:malloc + fun:malloc + fun:strdup + fun:_dl_load_cache_lookup +} +{ + dlopen() called without dlclose() + Memcheck:Leak + fun:malloc + fun:malloc + fun:strdup + fun:_dl_map_object +} +{ + dlopen() called without dlclose() + Memcheck:Leak + fun:malloc + fun:* + fun:_dl_new_object +} +{ + dlopen() called without dlclose() + Memcheck:Leak + fun:calloc + fun:* + fun:_dl_new_object +} +{ + dlopen() called without dlclose() + Memcheck:Leak + fun:calloc + fun:* + fun:_dl_check_map_versions +} + + # # Non-python specific leaks # @@ -2048,6 +2095,10 @@ fun:PyUnicode_FSConverter } +# Python Valgrind .supp end + + + # Actual GTK things { GtkWidgetClass action GPtrArray -- 2.49.1 From f636090fd9214743606e9baf23fd177f24b2a63f Mon Sep 17 00:00:00 2001 From: Jabber Developer Date: Wed, 2 Jul 2025 19:27:15 +0200 Subject: [PATCH 4/4] fix(window): preserve scroll offset on page_down and refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When paging down beyond the visible chat buffer, the scroll position could jump unexpectedly. This occurred because the visual distance from the bottom (e.g. 6 lines until the last message) wasn’t preserved after loading new messages from the DB. This patch fixes that by capturing the visual offset (`current_offset`) before the DB fetch and reapplying it after, based on the y-position of the final buffer entry. This ensures consistent and smooth scroll transitions, even as message heights vary. In addition to fixing the visual glitch, this commit introduces a targeted performance optimization: once the bottom of the chat has been reached (`WIN_SCROLL_REACHED_BOTTOM`), we skip expensive operations entirely — including datetime lookups and entry formatting — during further page_down events. This reduces performance stalls during rapid paging (e.g. holding the Page Down key at the bottom of the scrolling area). This update also adapts to the recent change in `chatwin_db_history()`, which now returns a `db_history_result_t` instead of `gboolean`, allowing correct differentiation between an empty result and an actual DB error. Together, these changes: - Fix a scroll-jump bug in `page_down` - Improve performance and responsiveness during rapid navigation - Align with improved DB result handling --- src/ui/window.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ui/window.c b/src/ui/window.c index 9bce0f7b..5dc81391 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -710,17 +710,21 @@ win_page_down(ProfWin* window, int scroll_size) // Scrolled down after reaching the bottom of the page if ((*page_start > total_rows - page_space || (*page_start == page_space && *page_start >= total_rows)) && window->type == WIN_CHAT) { int bf_size = buffer_size(window->layout->buffer); - if (bf_size > 0) { + if (bf_size > 0 && *scroll_state != WIN_SCROLL_REACHED_BOTTOM) { + // How many lines are left until end of the screen + int current_offset = total_rows - *page_start; GDateTime* now = g_date_time_new_now_local(); ProfBuffEntry* last_entry = buffer_get_entry(window->layout->buffer, bf_size - 1); auto_gchar gchar* start = g_date_time_format_iso8601(last_entry->time); auto_gchar gchar* end_date = g_date_time_format_iso8601(now); - if (*scroll_state != WIN_SCROLL_REACHED_BOTTOM && !chatwin_db_history((ProfChatWin*)window, start, end_date, FALSE)) { + db_history_result_t db_response = chatwin_db_history((ProfChatWin*)window, start, end_date, FALSE); + if (db_response == DB_RESPONSE_EMPTY) *scroll_state = WIN_SCROLL_REACHED_BOTTOM; - } - int offset = last_entry->y_end_pos - 1; - *page_start = offset - page_space + scroll_size; + // similar to page_up (see explanation there) + // Get offset of the message that was the latest prior to loading messages from the DB + int original_pos = last_entry->y_end_pos; + *page_start = original_pos - current_offset; g_date_time_unref(now); } } -- 2.49.1