changeset
Fix some broken logic in Purple.Conversation.send_message_async
Sat, 09 Aug 2025 02:43:31 -0500
- author
- Gary Kramlich <grim@reaperworld.com>
- date
- Sat, 09 Aug 2025 02:43:31 -0500
- changeset 43303
- cddf3066f1bc
- parent 43302
- e7b0bbfec5d5
- child 43304
- 2599d35e9750
- child 43305
- 4ede49515766
- child 43306
- 48da6dedaf1a
Fix some broken logic in Purple.Conversation.send_message_async
The callback was doing some nonsense trying to give the user an error if
Purple.ProtocolConversation.send_message_async returned false without setting
error. This was then clearing the error using g_clear_error even if error was
set, but G.Task.return_error takes ownership of the error which is what lead
to the crash.
This wasn't caught by the unit tests because we don't have a test for
purple_conversation_send_message_async and
purple_conversation_send_message_finish. We should add one at some point as we
also weren't exposing purple_conversation_send_message_finish which I
discovered while debugging this.
Anyways, we should implement a unit test for this but it needs a protocol
as well and I didn't want to bother with this right now as the fix is more
important.
Testing Done:
Modified the demo protocol to return errors and verified the crash was gone. Reverted that and verified that sending messages worked.
Also called in the turtles.
Reviewed at https://reviews.imfreedom.org/r/4095/
--- a/libpurple/purpleconversation.c Thu Aug 07 21:40:13 2025 -0500 +++ b/libpurple/purpleconversation.c Sat Aug 09 02:43:31 2025 -0500 @@ -324,23 +324,18 @@ success = purple_protocol_conversation_send_message_finish(protocol, result, &error); - if(!success) { - if(error == NULL) { - error = g_error_new(PURPLE_CONVERSATION_DOMAIN, 0, - "unknown error"); - } - + if(error != NULL) { purple_message_set_error(message, error); g_task_return_error(task, error); - - g_clear_error(&error); } else { - /* If the protocol didn't set delivered, set it now. */ - if(!purple_message_get_delivered(message)) { - purple_message_set_delivered(message, TRUE); + if(success) { + /* If the protocol didn't set delivered, set it now. */ + if(!purple_message_get_delivered(message)) { + purple_message_set_delivered(message, TRUE); + } } - g_task_return_boolean(task, TRUE); + g_task_return_boolean(task, success); } g_clear_object(&task); @@ -1704,8 +1699,9 @@ g_return_val_if_fail(PURPLE_IS_CONVERSATION(conversation), FALSE); g_return_val_if_fail(G_IS_ASYNC_RESULT(result), FALSE); - g_return_val_if_fail(g_task_get_source_tag(G_TASK(result)) != - purple_conversation_send_message_async, FALSE); + g_return_val_if_fail(g_async_result_is_tagged(result, + purple_conversation_send_message_async), + FALSE); return g_task_propagate_boolean(G_TASK(result), error); }
--- a/libpurple/purpleconversation.h Thu Aug 07 21:40:13 2025 -0500 +++ b/libpurple/purpleconversation.h Sat Aug 09 02:43:31 2025 -0500 @@ -385,6 +385,7 @@ * * Since: 3.0 */ +PURPLE_AVAILABLE_IN_3_0 gboolean purple_conversation_send_message_finish(PurpleConversation *conversation, GAsyncResult *result, GError **error); /**
--- a/pidgin/pidginconversation.c Thu Aug 07 21:40:13 2025 -0500 +++ b/pidgin/pidginconversation.c Sat Aug 09 02:43:31 2025 -0500 @@ -250,8 +250,8 @@ /* Send the message and clean up. We don't worry about the callback as we * don't have anything to do in it right now. */ - purple_conversation_send_message_async(conversation->conversation, message, - NULL, NULL, NULL); + purple_conversation_send_message_async(conversation->conversation, + message, NULL, NULL, NULL); g_clear_object(&message); }
