After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 677641 - Use a weak reference while loading the logs asynchronously to detect when the object has been destroyed
Use a weak reference while loading the logs asynchronously to detect when the...
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Chat
3.5.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-07 17:31 UTC by Debarshi Ray
Modified: 2012-06-11 11:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
empathy-chat: hold a reference while loading logs (928 bytes, patch)
2012-06-07 17:34 UTC, Debarshi Ray
reviewed Details | Review
empathy-chat: abort got_filtered_messages_cb if object was destroyed (1.52 KB, patch)
2012-06-08 14:27 UTC, Debarshi Ray
reviewed Details | Review
empathy-chat: abort chat_log_filter if object was destroyed (1.90 KB, patch)
2012-06-08 14:54 UTC, Debarshi Ray
reviewed Details | Review
empathy-chat: abort got_filtered_messages_cb if object was destroyed (1.55 KB, patch)
2012-06-11 11:14 UTC, Debarshi Ray
none Details | Review
empathy-chat: abort chat_log_filter if object was destroyed (2.29 KB, patch)
2012-06-11 11:14 UTC, Debarshi Ray
committed Details | Review
empathy-chat: abort got_filtered_messages_cb if object was destroyed (1.55 KB, patch)
2012-06-11 11:15 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2012-06-07 17:31:17 UTC
chat_add_logs gets invoked from the constructed method to asynchronously load the logs. Hold a reference to avoid hitting finalize before the callback is invoked.
Comment 1 Debarshi Ray 2012-06-07 17:34:07 UTC
Created attachment 215860 [details] [review]
empathy-chat: hold a reference while loading logs
Comment 2 Guillaume Desmottes 2012-06-08 06:30:55 UTC
Review of attachment 215860 [details] [review]:

The callback is pretty useless if the widget has been destroyed right? The logs won't be displayed. I think it's best to use TpWeakRef instead and so early return in the callback if the chat has been destroyed.

See for example http://git.gnome.org/browse/empathy/commit/?id=0e095b8af9cd07ae32823f73b889fdbfb47d6f2a
Comment 3 Debarshi Ray 2012-06-08 14:27:35 UTC
Created attachment 215965 [details] [review]
empathy-chat: abort got_filtered_messages_cb if object was destroyed
Comment 4 Debarshi Ray 2012-06-08 14:54:59 UTC
Created attachment 215967 [details] [review]
empathy-chat: abort chat_log_filter if object was destroyed
Comment 5 Guillaume Desmottes 2012-06-11 07:10:02 UTC
Review of attachment 215965 [details] [review]:

::: libempathy-gtk/empathy-chat.c
@@ +2550,3 @@
 	GList *messages;
+	TpWeakRef *wr = user_data;
+	EmpathyChat *chat = tp_weak_ref_dup_object (wr);

the ref on chat is leaked if the object isn't destroyed: dup_object returns a new ref.
Comment 6 Guillaume Desmottes 2012-06-11 07:10:36 UTC
Review of attachment 215967 [details] [review]:

::: libempathy-gtk/empathy-chat.c
@@ +2498,3 @@
 {
+	TpWeakRef *wr = user_data;
+	EmpathyChat *chat = tp_weak_ref_dup_object (wr);

same problem, chat is leaked.
Comment 7 Debarshi Ray 2012-06-11 11:14:17 UTC
Created attachment 216102 [details] [review]
empathy-chat: abort got_filtered_messages_cb if object was destroyed
Comment 8 Debarshi Ray 2012-06-11 11:14:41 UTC
Created attachment 216103 [details] [review]
empathy-chat: abort chat_log_filter if object was destroyed
Comment 9 Debarshi Ray 2012-06-11 11:15:37 UTC
Created attachment 216104 [details] [review]
empathy-chat: abort got_filtered_messages_cb if object was destroyed
Comment 10 Guillaume Desmottes 2012-06-11 11:18:50 UTC
++
Comment 11 Debarshi Ray 2012-06-11 11:21:01 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.