From 64046bcdf3433cef949f65bd8a8ba4c5a64e0575 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 14 Nov 2023 14:55:06 +0100 Subject: [PATCH 1/6] Fix some things regarding `char*` vs. `gchar*` Signed-off-by: Steffen Jaeckel --- src/xmpp/connection.c | 4 ++-- src/xmpp/message.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 925b047d..ca4d11bb 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -561,7 +561,7 @@ connection_disconnect(void) } } - free(prof_identifier); + g_free(prof_identifier); prof_identifier = NULL; } @@ -1126,7 +1126,7 @@ static void _compute_identifier(const char* barejid) { // in case of reconnect (lost connection) - free(prof_identifier); + g_free(prof_identifier); prof_identifier = g_compute_hmac_for_string(G_CHECKSUM_SHA256, (guchar*)profanity_instance_id, strlen(profanity_instance_id), diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 11efde65..867a7d0c 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -1261,7 +1261,7 @@ _handle_muc_private_message(xmpp_stanza_t* const stanza) ProfMessage* message = message_init(); message->type = PROF_MSG_TYPE_MUCPM; - const gchar* from = xmpp_stanza_get_from(stanza); + const char* from = xmpp_stanza_get_from(stanza); if (!from) { goto out; } @@ -1360,7 +1360,7 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c return; } - const gchar* from = xmpp_stanza_get_from(stanza); + const char* from = xmpp_stanza_get_from(stanza); if (!from) { return; } From ff32f51f4f6c0dd7809326521f639c6a633d00f8 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 14 Nov 2023 14:55:57 +0100 Subject: [PATCH 2/6] Fix libstrophe verbosity level always being set to 0 on start Fixup/revert of e55f6d7f4d831d370001999d55dafea9888a4a9f This line had been added because an earlier release of libstrophe missed the initialisation of that variable, which has since been fixed. Signed-off-by: Steffen Jaeckel --- src/xmpp/connection.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index ca4d11bb..91663422 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -190,7 +190,6 @@ _conn_apply_settings(const char* const jid, const char* const passwd, const char _compute_identifier(jidp->barejid); - xmpp_ctx_set_verbosity(conn.xmpp_ctx, 0); xmpp_conn_set_jid(conn.xmpp_conn, jid); if (passwd) xmpp_conn_set_pass(conn.xmpp_conn, passwd); From 2b51675e89e9e56df70fa9f0006d545b4dc8f5a8 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 14 Nov 2023 14:57:43 +0100 Subject: [PATCH 3/6] Save SM queue on `/reconnect now` Signed-off-by: Steffen Jaeckel --- src/xmpp/connection.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 91663422..71e7d21d 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -997,22 +997,25 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status log_debug("Connection handler: XMPP_CONN_DISCONNECT"); // lost connection for unknown reason - if (conn.conn_status == JABBER_CONNECTED) { + if (conn.conn_status == JABBER_CONNECTED || conn.conn_status == JABBER_DISCONNECTING) { if (prefs_get_boolean(PREF_STROPHE_SM_ENABLED)) { int send_queue_len = xmpp_conn_send_queue_len(conn.xmpp_conn); - log_debug("Connection handler: Lost connection for unknown reason"); + log_debug("Connection handler: Lost connection for unknown reason, %d messages in send queue", send_queue_len); conn.sm_state = xmpp_conn_get_sm_state(conn.xmpp_conn); if (send_queue_len > 0 && prefs_get_boolean(PREF_STROPHE_SM_RESEND)) { conn.queued_messages = calloc(send_queue_len + 1, sizeof(*conn.queued_messages)); for (int n = 0; n < send_queue_len && conn.queued_messages[n]; ++n) { conn.queued_messages[n] = xmpp_conn_send_queue_drop_element(conn.xmpp_conn, XMPP_QUEUE_OLDEST); } + } else if (send_queue_len > 0) { + log_debug("Connection handler: dropping those messages since SM RESEND is disabled"); } } - session_lost_connection(); + if (conn.conn_status == JABBER_CONNECTED) + session_lost_connection(); // login attempt failed - } else if (conn.conn_status != JABBER_DISCONNECTING) { + } else { gchar* host; int port; if (stream_error && stream_error->stanza && _get_other_host(stream_error->stanza, &host, &port)) { From c3ed0c3262c74cc1e7ab92058712bf8bee66ea61 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 16 Nov 2023 13:19:28 +0100 Subject: [PATCH 4/6] Only handle MAM response if window still exists Before this patch the following scenario lead to a segfault: 1. open a window that sends a MAM request 2. fast enough close that window again before the MAM response was processed Once the MAM response is received we'd call `_mam_rsm_id_handler()` from the `_iq_handler()` and `window` would point to a non-existant window which leads to a segfault. Signed-off-by: Steffen Jaeckel --- src/ui/core.c | 2 ++ src/xmpp/iq.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ src/xmpp/xmpp.h | 1 + 3 files changed, 55 insertions(+) diff --git a/src/ui/core.c b/src/ui/core.c index 24e15eba..39535eaa 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -712,6 +712,8 @@ ui_close_win(int index) } } + // remove the IQ handlers + iq_handlers_remove_win(window); wins_close_by_num(index); title_bar_console(); status_bar_current(1); diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index 793dbc59..924c98be 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -267,6 +267,54 @@ iq_handlers_init(void) rooms_cache = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)xmpp_stanza_release); } +struct iq_win_finder +{ + gsize max, cur; + char** to_be_removed; +}; + +static void +_win_find(char* key, + ProfIqHandler* handler, + struct iq_win_finder* finder) +{ + if (handler->func == _mam_rsm_id_handler) { + if (finder->cur >= finder->max) { + finder->max *= 2; + finder->to_be_removed = g_realloc_n(finder->to_be_removed, finder->max, sizeof(char*)); + } + finder->to_be_removed[finder->cur++] = g_strdup(key); + } +} + +void +iq_handlers_remove_win(ProfWin* window) +{ + log_debug("Remove window %p of type %d", window, window ? window->type : -1); + if (!window) + return; + GSList *cur = late_delivery_windows, *next; + while (cur) { + LateDeliveryUserdata* del_data = cur->data; + next = g_slist_next(cur); + if (del_data->win == (void*)window) + late_delivery_windows = g_slist_delete_link(late_delivery_windows, + cur); + cur = next; + } + struct iq_win_finder st = { 0 }; + st.max = g_hash_table_size(id_handlers); + if (st.max == 0) + return; + st.to_be_removed = g_new(char*, st.max); + g_hash_table_foreach(id_handlers, (GHFunc)_win_find, &st); + for (gsize n = 0; n < st.cur; ++n) { + g_hash_table_remove(id_handlers, st.to_be_removed[n]); + g_free(st.to_be_removed[n]); + } + g_free(st.to_be_removed); +} + void iq_handlers_clear() { @@ -2696,6 +2744,10 @@ _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata) gboolean is_complete = g_strcmp0(xmpp_stanza_get_attribute(fin, "complete"), "true") == 0; MamRsmUserdata* data = (MamRsmUserdata*)userdata; ProfWin* window = (ProfWin*)data->win; + if (wins_get_num(window) == -1) { + log_error("Window %p should not get any events anymore", window); + return 0; + } buffer_remove_entry(window->layout->buffer, 0); diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index baff1295..23b20729 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -240,6 +240,7 @@ void iq_enable_carbons(void); void iq_disable_carbons(void); void iq_send_software_version(const char* const fulljid); void iq_rooms_cache_clear(void); +void iq_handlers_remove_win(ProfWin* window); void iq_handlers_clear(); void iq_room_list_request(gchar* conferencejid, gchar* filter); void iq_disco_info_request(gchar* jid); From 629cd33e2f4896e9cf41aa2c5202c4debd51d27a Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 16 Nov 2023 13:29:08 +0100 Subject: [PATCH 5/6] Reset internal flag Reset the `received_disco_items` flag when initializing the iq module. This has caused the console error message "Server doesn't support MAM" sometimes on reconnect. Fixes #1940 Signed-off-by: Steffen Jaeckel --- src/xmpp/iq.c | 3 ++- src/xmpp/xmpp.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index 924c98be..cd310cc3 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -259,6 +259,7 @@ iq_handlers_init(void) int millis = prefs_get_autoping() * 1000; xmpp_timed_handler_add(conn, _autoping_timed_send, millis, ctx); } + received_disco_items = FALSE; iq_rooms_cache_clear(); iq_handlers_clear(); @@ -316,7 +317,7 @@ iq_handlers_remove_win(ProfWin* window) } void -iq_handlers_clear() +iq_handlers_clear(void) { if (id_handlers) { g_hash_table_remove_all(id_handlers); diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index 23b20729..494d5fc1 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -241,7 +241,7 @@ void iq_disable_carbons(void); void iq_send_software_version(const char* const fulljid); void iq_rooms_cache_clear(void); void iq_handlers_remove_win(ProfWin* window); -void iq_handlers_clear(); +void iq_handlers_clear(void); void iq_room_list_request(gchar* conferencejid, gchar* filter); void iq_disco_info_request(gchar* jid); void iq_disco_items_request(gchar* jid); From 31c6d5f09a74b57d6a368926b3a1a55861c1e4b2 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 17 Nov 2023 13:49:48 +0100 Subject: [PATCH 6/6] Improve shutdown 1. close logfile as last action 2. Fix `plugins_shutdown()` accessing `((ProfPlugin*)curr->data)->lang` after `curr->data` had already potentially been free'd. Signed-off-by: Steffen Jaeckel --- src/plugins/plugins.c | 12 +++++++----- src/profanity.c | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/plugins/plugins.c b/src/plugins/plugins.c index 3b64c7aa..274b711e 100644 --- a/src/plugins/plugins.c +++ b/src/plugins/plugins.c @@ -932,21 +932,23 @@ void plugins_shutdown(void) { GList* values = g_hash_table_get_values(plugins); - GList* curr = values; + GList *curr = values, *next; while (curr) { + next = g_list_next(curr); #ifdef HAVE_PYTHON - if (((ProfPlugin*)curr->data)->lang == LANG_PYTHON) { + if (curr && ((ProfPlugin*)curr->data)->lang == LANG_PYTHON) { python_plugin_destroy(curr->data); + curr = NULL; } #endif #ifdef HAVE_C - if (((ProfPlugin*)curr->data)->lang == LANG_C) { + if (curr && ((ProfPlugin*)curr->data)->lang == LANG_C) { c_plugin_destroy(curr->data); + curr = NULL; } #endif - - curr = g_list_next(curr); + curr = next; } g_list_free(values); #ifdef HAVE_PYTHON diff --git a/src/profanity.c b/src/profanity.c index 0731d711..9660945c 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -256,9 +256,9 @@ _shutdown(void) accounts_close(); tlscerts_close(); log_stderr_close(); - log_close(); plugins_shutdown(); cmd_uninit(); ui_close(); prefs_close(); + log_close(); }