Bug 343447

Meeks I have never experienced this. (Good you have traces this time :)

Sankar, Can you look into what is happening here? I suspect some sort of missing signal disconnects/recursion or something.

Well ;-) it's very common here - and it makes using evo HEAD perform like wading through sticky mud ;-)

I just got annoyed enough with a similar typing break to debug this again:

#11 0xb60ff874 in em_folder_tree_model_set_unread_count (model=0x808ad90, store=0x8689d08, full=0xb1a56d58 "Mail/0ximian-desktop/NLD/suse-research", 
    unread=9727) at em-folder-tree-model.c:1187
#12 0xb613a58d in real_flush_updates (o=0x0, event_data=0x0, data=0x0) at mail-folder-cache.c:232
#13 0xb613bd63 in do_async_event (mm=0xb0b01040) at mail-mt.c:688

ie. a different folder this time.

What is most irritating is that (presumably) this should simply change a single unread-count / label in the tree model visible on the left hand side: ie. this should be absolutely instant ! ;-)

Unfortunately, instead it takes tens of seconds to do [ for each time we update an unread count - which happens quite a bit ] ;-)

Soo ... reading the code here:

mail/em-folder-tree.c (emft_model_changed)

The trace seems impossible; at least it's not clear to me how we can get from set_unread_count to:

#24 0xb703e720 in IA__gtk_tree_model_row_changed (tree_model=0x808ad90,
path=0xae0e8250, iter=0xbfb33bcc) at gtktreemodel.c:1476
#25 0xb60bc35c in emft_model_row_changed (model=0x808ad90, path=0xaeb030c0,
iter=0xae5c6ba0, emft=0x98df5a0) at em-folder-tree.c:1864

to

#17 0xb703e720 in IA__gtk_tree_model_row_changed (tree_model=0x808ad90,
path=0xaeb02ce8, iter=0xbfb336dc) at gtktreemodel.c:1476
#18 0xb60bc35c in emft_model_row_changed (model=0x808ad90, path=0xae0e8250,
iter=0x96aa290, emft=0xaef32150) at em-folder-tree.c:1864
to

#3  0xb703e720 in IA__gtk_tree_model_row_changed (tree_model=0x808ad90,
path=0xae0bd7b0, iter=0xbfb32cfc) at gtktreemodel.c:1476
#4  0xb60bc35c in emft_model_row_changed (model=0x808ad90, path=0xae0bd160,
iter=0xb696aa0, emft=0x994e650) at em-folder-tree.c:1864

Since we block the row_changed signal emission.

On the other hand - it seems we have a different EmFolderTree *emft each time - which explains it, but -how- does that happen ? :-)

Haha! - so, I speculate here:

It looks (to me) like the bug lurks at: em_folder_tree_construct:

	g_signal_connect (priv->model, "row-changed", G_CALLBACK (emft_model_row_changed), emft);

* Why are we connecting a signal handler to an object, and then not life-cycle managing it properly ?
* Can we guarentee that this model has only one view ?
* If so, why do we not construct it ourselves ?
* *Why!* are we not removing that signal on destruction of emft ?
    + [ that looks really dangerous, either a big leak or a crasher bug ].
* *Why!* are we manipulating the model in the view when this is (clearly) a model-related issue ? can we not do this in the model instead ? [ surely that makes more sense ]

All calls of em_folder_tree_new_with_model are here:

em-folder-selection-button.c:236:	emft = (EMFolderTree *) em_folder_tree_new_with_model (model);
em-folder-selection.c:67:	emft = (EMFolderTree *) em_folder_tree_new_with_model (model);
em-folder-tree.c:568:	emft = (EMFolderTree *) em_folder_tree_new_with_model (model);
em-folder-tree.c:706:em_folder_tree_new_with_model (EMFolderTreeModel *model)
em-folder-tree.h:73:GtkWidget *em_folder_tree_new_with_model (EMFolderTreeModel *model);
em-folder-utils.c:748:	folder_tree = (EMFolderTree *) em_folder_tree_new_with_model (model);
em-vfolder-rule.c:509:	emft =(EMFolderTree *)em_folder_tree_new_with_model(mail_component_peek_tree_model(mail_component_peek()));
mail-component.c:702:	tree_widget = (GtkWidget *) em_folder_tree_new_with_model (priv->model);

Presumably one or other of them is (sensibly) re-using the model [ to reduce memory use ], but the view is clinging to it (somehow).

I normally create 3x evo. windows in mail mode, then switch them one by one to addressbook, then calendar; I also use C-M-v to move mails around (another folder view) a lot: I guess it's the top-level that is not lifecycle managed properly though rather than C-M-v.

HTH.

Good news - I got another hang, and was annoyed enough to see if it was the same thing: it was, this time I poked around at the emft's to see what was up there:

#3  0xb60ee392 in emft_model_row_changed (model=0x808ad90, path=0xb7c5688, iter=0xb726280, emft=0xb4b9640) at em-folder-tree.c:1860
1860            while (gtk_tree_model_iter_parent (model, &parent_iter, &child_iter)) {
(gdb) p *emft
$1 = {parent_object = {box = {container = {widget = {object = {parent_instance = {g_type_instance = {g_class = 0xb}, ref_count = 0, qdata = 0x0}, 
            flags = 0}, private_flags = 12703, state = 166 '¦', saved_state = 103 'g', name = 0x0, style = 0x0, requisition = {width = 0, height = 0}, 
          allocation = {x = 0, y = 0, width = 0, height = 0}, window = 0x0, parent = 0x0}, focus_child = 0x0, border_width = 0, need_resize = 0, 
        resize_mode = 0, reallocate_redraws = 0, has_focus_chain = 0}, children = 0x0, spacing = 0, homogeneous = 0}}, priv = 0xb0595e8}
(gdb) p *emft->priv
$2 = {treeview = 0x2e626f72, model = 0x6c796174, select_uris = 0x6340726f, select_uris_table = 0x616c6c6f, excluded = 1634889570, 
  excluded_func = 0x2e6f632e, excluded_data = 0x3006b75, do_multiselect = 1, cursor_set = 0, save_state_id = 4475221, autoscroll_id = 3061240200, 
  autoexpand_id = 174393296, autoexpand_row = 0x11, loading_row_id = 5653842, loaded_row_id = 3061240184, drag_row = 0x10}

looks ok - but the next frame up:

#10 0xb60ee35c in emft_model_row_changed (model=0x808ad90, path=0xabe3508, iter=0xb1fb5480, emft=0xaaa1ae0) at em-folder-tree.c:1864
1864                    gtk_tree_model_row_changed (model, parent_path, &parent_iter);
(gdb) p *emft
$3 = {parent_object = {box = {container = {widget = {object = {parent_instance = {g_type_instance = {g_class = 0xb}, ref_count = 0, qdata = 0x0}, 
            flags = 0}, private_flags = 46808, state = 94 '^', saved_state = 103 'g', name = 0x0, style = 0x0, requisition = {width = 0, height = 0}, 
          allocation = {x = 0, y = 0, width = 0, height = 0}, window = 0x0, parent = 0x0}, focus_child = 0x0, border_width = 0, need_resize = 0, 
        resize_mode = 0, reallocate_redraws = 0, has_focus_chain = 0}, children = 0x0, spacing = 0, homogeneous = 0}}, priv = 0xa686438}
(gdb) p *emft->priv
$4 = {treeview = 0x0, model = 0x3, select_uris = 0xb6dfaa00, select_uris_table = 0x0, excluded = 0, excluded_func = 0x80e8830, excluded_data = 0x0, 
  do_multiselect = 0, cursor_set = 0, save_state_id = 135205720, autoscroll_id = 0, autoexpand_id = 166431672, autoexpand_row = 0x0, loading_row_id = 0, 
  loaded_row_id = 0, drag_row = 0x0}

Looks badly duff - a 'model' of 0x3 ;-) - anyhow I guess the good news is we're not leaking: just not using the emft - except as a random number:

#17 0xb60ee35c in emft_model_row_changed (model=0x808ad90, path=0xab39f18, iter=0xb0955c60, emft=0xb13b458) at em-folder-tree.c:1864
1864                    gtk_tree_model_row_changed (model, parent_path, &parent_iter);
(gdb) p *emft
$5 = {parent_object = {box = {container = {widget = {object = {parent_instance = {g_type_instance = {g_class = 0xb}, ref_count = 0, qdata = 0x0}, 
            flags = 0}, private_flags = 35135, state = 96 '`', saved_state = 103 'g', name = 0x0, style = 0x0, requisition = {width = 0, height = 0}, 
          allocation = {x = 0, y = 0, width = 0, height = 0}, window = 0x0, parent = 0x0}, focus_child = 0x0, border_width = 0, need_resize = 0, 
        resize_mode = 0, reallocate_redraws = 0, has_focus_chain = 0}, children = 0x0, spacing = 0, homogeneous = 0}}, priv = 0x98f7e18}
(gdb) p *emft->priv
$6 = {treeview = 0x0, model = 0x3, select_uris = 0xb6dfaa00, select_uris_table = 0x0, excluded = 0, excluded_func = 0x810cb58, excluded_data = 0x0, 
  do_multiselect = 0, cursor_set = 0, save_state_id = 135205720, autoscroll_id = 0, autoexpand_id = 191355872, autoexpand_row = 0x0, loading_row_id = 0, 
  loaded_row_id = 0, drag_row = 0x0}

just the same.

I append this patch fragment as an *example* of how not to do it - this should fix the proximate cause: but the underlying bug is structural - why !? is this change notification in the view, and not in the model ? :-)

--- em-folder-tree.c
+++ em-folder-tree.c
@@ -481,6 +481,10 @@
                priv->autoexpand_id = 0;
        }
 
+       g_signal_handlers_disconnect_by_func
+               (priv->model, "row-changed",
+                G_CALLBACK (emft_model_row_changed), emft);
+
        priv->treeview = NULL;
        priv->model = NULL;
 

Fixing it more pleasantly should be easy, but if it's beyond doing - then I'd really like a usable mail-client soon, even if it has to be done like the above ;-)

woudln't that bug be also http://bugzilla.gnome.org/show_bug.cgi?id=483017

Created attachment 185772 [details]
Likely Fix

May be this patch should fix the issue. As I never get the CPU usage (and not sure if I have understood the code fully), I am unable to verify this :( 

Can you review/verify this ? 

Srini: Is there any other reason/scenario than changing unread count that will need the code in row_changed to be run ?

patch looks great to me :-) should work fine. I'd personally be inclined to follow row_changed as before if we're not sure [ pwrt. a SL10.3 update ].
Looking forward to seeing it...

Created attachment 186766 [details]
Michael Meeks' fix

Anja, Can you give a SWAMP id for this bug ? We want to commit this to 10.3

I don't think we want to release an update for this specific problem, but if the evo team is fine with it, we can hook it into the next update.

Anja: I do not know about the timezone patch atm. I will try to consolidate all the patches that should go for 10.3 and will get a single SWAMP id from you. I will do this in a day or two.

Patch is submitted for this and committed. Please check for updates.

released

(In reply to comment #6 from Hubert Figuiere)
> woudln't that bug be also http://bugzilla.gnome.org/show_bug.cgi?id=483017
> 

The GNOME bug is no longer reproductible with that fix. That was it.

Thanks Hubert for verifying the fix. And thanks to Michael for the initial idea about the CPU usage. 

*** Bug 340265 has been marked as a duplicate of this bug. ***