From 7d9c3c1b32fcf0804a384b80a3e205b646a69b71 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 14 Apr 2023 13:18:58 +0200 Subject: [PATCH 1/4] fix "window `NULL` issue" (hopefully) There were multiple reports where after a reconnect the window of the MUC that was last opened, was empty. `muc_join()` creates an instance of a MUC, `presence_join_room()` works with this instance. Therefore the instance has to exist before working on it. I'm not sure if this really fixes the issue, but at least it didn't happen anymore after I applied this modification. I can't remember how I stumbled over this, either while looking at debug logs or while looking at Valgrind output while a reconnect happened, but something went wrong. Then I came to the conclusion that this may fix the issue and for now it did ... maybe it comes back, then my RCA was wrong. Signed-off-by: Steffen Jaeckel --- src/event/server_events.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/event/server_events.c b/src/event/server_events.c index 0f6df2ac..89eefef6 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -1332,8 +1332,8 @@ sv_ev_bookmark_autojoin(Bookmark* bookmark) log_debug("Autojoin %s with nick=%s", bookmark->barejid, nick); if (!muc_active(bookmark->barejid)) { - presence_join_room(bookmark->barejid, nick, bookmark->password); muc_join(bookmark->barejid, nick, bookmark->password, TRUE); + presence_join_room(bookmark->barejid, nick, bookmark->password); iq_room_affiliation_list(bookmark->barejid, "member", false); iq_room_affiliation_list(bookmark->barejid, "admin", false); iq_room_affiliation_list(bookmark->barejid, "owner", false); From 08d68d329bac4417a8c7a9c44d7398e460fc0751 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Wed, 10 May 2023 11:08:44 +0200 Subject: [PATCH 2/4] more `auto_char` Signed-off-by: Steffen Jaeckel --- src/event/client_events.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/event/client_events.c b/src/event/client_events.c index 67ffec20..4dcda4f3 100644 --- a/src/event/client_events.c +++ b/src/event/client_events.c @@ -85,9 +85,8 @@ cl_ev_connect_account(ProfAccount* account) void cl_ev_disconnect(void) { - char* mybarejid = connection_get_barejid(); + auto_char char* mybarejid = connection_get_barejid(); cons_show("%s logged out successfully.", mybarejid); - free(mybarejid); ui_close_all_wins(); ev_disconnect_cleanup(); From 8cd53acfd7450b4c1d22c956021af28135c38240 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Wed, 10 May 2023 15:07:07 +0200 Subject: [PATCH 3/4] fix `/reconnect now` This fixes #1846 Issue introduced by a0aa26b6fa65ba625f4a6d3495a345c7120ff16d Signed-off-by: Steffen Jaeckel --- src/command/cmd_funcs.c | 3 ++- src/event/client_events.c | 13 +++++++++++++ src/event/client_events.h | 1 + src/xmpp/connection.h | 1 - src/xmpp/session.h | 1 - src/xmpp/xmpp.h | 2 ++ tests/unittests/xmpp/stub_xmpp.c | 4 ++++ 7 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 6e9ca259..7f44c116 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -6575,7 +6575,8 @@ cmd_reconnect(ProfWin* window, const char* const command, gchar** args) int intval = 0; char* err_msg = NULL; if (g_strcmp0(value, "now") == 0) { - session_reconnect_now(); + cons_show("Reconnecting now."); + cl_ev_reconnect(); } else if (strtoi_range(value, &intval, 0, INT_MAX, &err_msg)) { prefs_set_reconnect(intval); if (intval == 0) { diff --git a/src/event/client_events.c b/src/event/client_events.c index 4dcda4f3..c393ccd4 100644 --- a/src/event/client_events.c +++ b/src/event/client_events.c @@ -47,6 +47,7 @@ #include "plugins/plugins.h" #include "ui/window_list.h" #include "xmpp/chat_session.h" +#include "xmpp/session.h" #include "xmpp/xmpp.h" #ifdef HAVE_LIBOTR @@ -94,6 +95,18 @@ cl_ev_disconnect(void) ev_reset_connection_counter(); } +void +cl_ev_reconnect(void) +{ + if (connection_get_status() != JABBER_DISCONNECTED) { + connection_disconnect(); + ev_disconnect_cleanup(); + // on intentional disconnect reset the counter + ev_reset_connection_counter(); + } + session_reconnect_now(); +} + void cl_ev_presence_send(const resource_presence_t presence_type, const int idle_secs) { diff --git a/src/event/client_events.h b/src/event/client_events.h index fed2bb37..a35e97b5 100644 --- a/src/event/client_events.h +++ b/src/event/client_events.h @@ -42,6 +42,7 @@ jabber_conn_status_t cl_ev_connect_jid(const char* const jid, const char* const jabber_conn_status_t cl_ev_connect_account(ProfAccount* account); void cl_ev_disconnect(void); +void cl_ev_reconnect(void); void cl_ev_presence_send(const resource_presence_t presence_type, const int idle_secs); diff --git a/src/xmpp/connection.h b/src/xmpp/connection.h index 79bee1d4..d4ade03a 100644 --- a/src/xmpp/connection.h +++ b/src/xmpp/connection.h @@ -48,7 +48,6 @@ jabber_conn_status_t connection_connect(const char* const fulljid, const char* c const char* const tls_policy, const char* const auth_policy); jabber_conn_status_t connection_register(const char* const altdomain, int port, const char* const tls_policy, const char* const username, const char* const password); -void connection_disconnect(void); void connection_set_disconnected(void); void connection_set_priority(const int priority); diff --git a/src/xmpp/session.h b/src/xmpp/session.h index e6facb93..d8565fa4 100644 --- a/src/xmpp/session.h +++ b/src/xmpp/session.h @@ -47,6 +47,5 @@ void session_init_activity(void); void session_check_autoaway(void); void session_reconnect(gchar* altdomain, unsigned short altport); -void session_reconnect_now(void); #endif diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index 539126cd..2babe536 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -186,7 +186,9 @@ void session_disconnect(void); void session_shutdown(void); void session_process_events(void); char* session_get_account_name(void); +void session_reconnect_now(void); +void connection_disconnect(void); jabber_conn_status_t connection_get_status(void); char* connection_get_presence_msg(void); void connection_set_presence_msg(const char* const message); diff --git a/tests/unittests/xmpp/stub_xmpp.c b/tests/unittests/xmpp/stub_xmpp.c index a7dc9ebf..ffa7565d 100644 --- a/tests/unittests/xmpp/stub_xmpp.c +++ b/tests/unittests/xmpp/stub_xmpp.c @@ -54,6 +54,10 @@ void session_process_events(void) { } +void +connection_disconnect(void) +{ +} const char* connection_get_fulljid(void) { From 8d3c1f79ac7cc2b0830f0afed48dc1fb9008ab0e Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Wed, 10 May 2023 16:32:04 +0200 Subject: [PATCH 4/4] fix memory leaks Signed-off-by: Steffen Jaeckel --- src/xmpp/connection.c | 2 ++ src/xmpp/iq.c | 41 ++++++++++++++++++++--------------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index eef395f1..2a022cc4 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -964,6 +964,7 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status conn.domain = strdup(my_jid->domainpart); jid_destroy(my_jid); + connection_clear_data(); conn.features_by_jid = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); g_hash_table_insert(conn.features_by_jid, strdup(conn.domain), g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL)); @@ -990,6 +991,7 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status conn.domain = strdup(my_raw_jid->domainpart); jid_destroy(my_raw_jid); + connection_clear_data(); conn.features_by_jid = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); g_hash_table_insert(conn.features_by_jid, strdup(conn.domain), g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL)); diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index f09deead..4f6eea40 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -265,6 +265,7 @@ iq_handlers_init(void) xmpp_timed_handler_add(conn, _autoping_timed_send, millis, ctx); } + iq_rooms_cache_clear(); iq_handlers_clear(); id_handlers = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_iq_id_handler_free); @@ -2651,6 +2652,18 @@ iq_mam_request_older(ProfChatWin* win) return; } +static void +_mam_userdata_free(MamRsmUserdata* data) +{ + free(data->end_datestr); + data->end_datestr = NULL; + free(data->start_datestr); + data->start_datestr = NULL; + free(data->barejid); + data->barejid = NULL; + free(data); +} + void _iq_mam_request(ProfChatWin* win, GDateTime* startdate, GDateTime* enddate) { @@ -2694,7 +2707,7 @@ _iq_mam_request(ProfChatWin* win, GDateTime* startdate, GDateTime* enddate) data->fetch_next = fetch_next; data->win = win; - iq_id_handler_add(xmpp_stanza_get_id(iq), _mam_rsm_id_handler, NULL, data); + iq_id_handler_add(xmpp_stanza_get_id(iq), _mam_rsm_id_handler, (ProfIqFreeCallback)_mam_userdata_free, data); } iq_send_stanza(iq); @@ -2742,13 +2755,13 @@ _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata) buffer_remove_entry(window->layout->buffer, 0); - char* start_str = NULL; + auto_char char* start_str = NULL; if (data->start_datestr) { start_str = strdup(data->start_datestr); // Convert to iso8601 start_str[strlen(start_str) - 3] = '\0'; } - char* end_str = NULL; + auto_char char* end_str = NULL; if (data->end_datestr) { end_str = strdup(data->end_datestr); // Convert to iso8601 @@ -2757,24 +2770,10 @@ _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata) if (is_complete || !data->fetch_next) { chatwin_db_history(data->win, is_complete ? NULL : start_str, end_str, TRUE); - // TODO free memory - if (start_str) { - free(start_str); - free(data->start_datestr); - } - - if (end_str) { - free(data->end_datestr); - } - - free(data->barejid); - free(data); return 0; } chatwin_db_history(data->win, start_str, end_str, TRUE); - if (start_str) - free(start_str); xmpp_stanza_t* set = xmpp_stanza_get_child_by_name_and_ns(fin, STANZA_TYPE_SET, STANZA_NS_RSM); if (set) { @@ -2787,14 +2786,14 @@ _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata) // 4.3.2. send same stanza with set,max stanza xmpp_ctx_t* const ctx = connection_get_ctx(); - if (end_str) { + if (data->end_datestr) { free(data->end_datestr); + data->end_datestr = NULL; } - data->end_datestr = NULL; - xmpp_stanza_t* iq = stanza_create_mam_iq(ctx, data->barejid, data->start_datestr, data->end_datestr, firstid, NULL); + xmpp_stanza_t* iq = stanza_create_mam_iq(ctx, data->barejid, data->start_datestr, NULL, firstid, NULL); free(firstid); - iq_id_handler_add(xmpp_stanza_get_id(iq), _mam_rsm_id_handler, NULL, data); + iq_id_handler_add(xmpp_stanza_get_id(iq), _mam_rsm_id_handler, (ProfIqFreeCallback)_mam_userdata_free, data); iq_send_stanza(iq); xmpp_stanza_release(iq);