tag rename hangs

[Michael Dance]

Running Getting Things Gnome! 0.2 it has happened twice now when trying to rename a tag from @help to @delegate the application freezes and I have to kill it

[Luca Invernizzi (invernizzi)]

Thanks for the bug report, Michael.
 In order to tackle the problem, we'll need some more informations from you.
First, please start gtg from the command line, reproduce the bug and post here the log of the application freezing.
If you can list a series of steps to reproduce the bug it would be great, but I believe that is only happening in your particular configuration.

[Michael Dance (michael-dance)]

Hey Luca,

I'm not sure if perhaps I just needed to restart, but I am unable to reproduce the issue now... Will keep my eyes peeled if anything comes up!

[Lionel Dricot (ploum-deactivatedaccount)]

Marking this as incomplete then. Please reopen the bug as soon as you can reproduce it. (I hope it's not a threading bug but I don't see why threads would be involved in the tag renaming)

[Lionel Dricot (ploum-deactivatedaccount)]

This is how to reproduce the hang :

1) select a tag in the browser
2) open one of task with that tag
3) from within the browser, rename the tag

Note that the tag is not changed in the editor. It should.

4) Close the task.

see that the old name reappear (logical, it was in the closed task)

5) clic on the old name in the browser then back on the new one

BADABOUM

[Lionel Dricot (ploum-deactivatedaccount)]

Fix to bug #497397 have made this one easier to reproduce : simply rename a tag affected to an opened task.

[Lionel Dricot (ploum-deactivatedaccount)]

This, in fact, happens every an opened task is modified externally. Like if you rename a subtask with that subtask already opened.

This is a pretty important bug IMHO.

I don't know if we should make it 0.2.1 release critical or not

[Bertrand Rousseau (bertrand-rousseau)]

I think we should. I think it's more important to provide something
that works rather than something more powerful. Don't know if it's
hard to fix, tough.

On Thu, Jan 7, 2010 at 6:12 PM, Lionel Dricot <email address hidden> wrote:
> This, in fact, happens every an opened task is modified externally. Like
> if you rename a subtask with that subtask already opened.
>
> This is a pretty important bug IMHO.
>
> I don't know if we should make it 0.2.1 release critical or not
>
> --
> tag rename hangs
> https://bugs.launchpad.net/bugs/502902
> You received this bug notification because you are subscribed to Getting
> Things GNOME!.
>

--
Bertrand Rousseau

[Matthew Rasmus (mrasmus)]

I was playing around with this bug a bit and came up with a few observations that I hope are helpful: this bug DOESN'T seem to happen every time an opened task is modified externally. You can create a new task, open it, and then drag and drop it to a few tags. The addition of these tags doesn't update in the taskeditor, but if you close it and reopen it it should look fine. After this initial step I did find a new bug (which I'll fill out a report for as soon as I'm done here) : with the task open, adding new tags via drag and drop will NOT update the task with those tags when it is closed and reopened, but it will show up under those tags (similar to the way that tasks that cause the hang will still only have the original tag name but show up in the renamed tag). I haven't yet actually gone through and looked at how the renaming process works, but I'm guessing that it removes the original tag and replaces it with the new one. Given that adding tags externally doesn't seem to cause the hang, that leads me to believe that the problem is in the _removal_ of tags externally.

[Bertrand Rousseau (bertrand-rousseau)]

I just spent several hours trying to figure out what this bug is all about. This a very tricky one. I think it is related to the fact that our TreeModels and our TagStore start to diverge when we rename the tag.

Since all treeviews relie on GtkTreeModels primitives to display the rows, once a tag is renamed, all hell breaks loose. Right now, I just can't figure how we could easily fix this, since I haven't clearly identified where and why GTG starts to loop indefinitely.

So, as for the 0.2.1 release that leave us two choices:

 1 - we disable tag renaming, so everything works correctly, but there's a feature regression, and we try to fix this for the next release.
 2 - we leave tag renaming, but we have a major bug, and we try to fix this for the next release.

I clearly prefer having a less feature-packed app than something that doesn't work, so I would vote for solution 1.

As a side remarks this kind of problems make me strongly believe that there is something wrong with our data structures.

[Lionel Dricot (ploum-deactivatedaccount)]

I've tried to go back to the root of the loop. The lower I've found is "tag_visible_func" in browser.py.

That particular function is definitely part of the loop but I can't figure out what can call it. It's definitely not the "refilter" methods. So, why is this function called ?

IMPORTANT :

I've found that our problem is mainly caused by the tags being stored both in a list and in a tree in the tagstore and in the tagtreeview ! The problem is that this particular tree is out-of-sync with the tags dic when we rename the tag. We should clearly dismiss the dic in favour of using the tree only.

I changed the way rename works to make it a "create new tag then remove the old one". I changed the rename function to also remove the old tag in the tree and the loop disapear ! (it's very buggy because then the tree is pointing on a NULL tag )

My code is of no interest, IMHO, but this is the way to go. Kevin, could you have a look at it as it's mainly your code. I don't understand some stuffs in your Tree class and I add to manually create my own "remove_byid" function.

[Bertrand Rousseau (bertrand-rousseau)]

On Mon, Jan 18, 2010 at 4:53 PM, Lionel Dricot <email address hidden> wrote:
> I've tried to go back to the root of the loop. The lower I've found is
> "tag_visible_func" in browser.py.
>
> That particular function is definitely part of the loop but I can't
> figure out what can call it. It's definitely not the "refilter" methods.
> So, why is this function called ?

The refilter function actually tries the visible_func callback (here:
tag_visible_func) on all element of the model.

>
> IMPORTANT :
>
> I've found that our problem is mainly caused by the tags being stored
> both in a list and in a tree in the tagstore and in the tagtreeview !
> The problem is that this particular tree is out-of-sync with the tags
> dic when we rename the tag. We should clearly dismiss the dic in favour
> of using the tree only.

Sounds good to me.

> I changed the way rename works to make it a "create new tag then remove
> the old one". I changed the rename function to also remove the old tag
> in the tree and the loop disapear !  (it's very buggy because then the
> tree is pointing on a NULL tag )

Good news, so that confirms what I thought.

> My code is of no interest, IMHO, but this is the way to go. Kevin, could
> you have a look at it as it's mainly your code. I don't understand some
> stuffs in your Tree class and I add to manually create my own
> "remove_byid" function.

The Tree class is actually my code, it's the base structure behinf all
the GenericTreeModels we use for the GtkTreeViews. Using it it
globally in the tagstore and the treemodel would probably severely
consolidate things.

> --
> tag rename hangs
> https://bugs.launchpad.net/bugs/502902
> You received this bug notification because you are subscribed to Getting
> Things GNOME!.
>

--
Bertrand Rousseau

[Lionel Dricot (ploum-deactivatedaccount)]

just added my really ugly branch, in case you are interested.

[Kevin Mehall (kevin-mehall)]

Ploum's solution of moving the tasks to a new tag instead of modifying it looks good to me. The old way did a bunch of redundant work. Just remember to handle subtags properly. I don't think removing the tag from the tree is necessary; remove all its attributes and it will go away when it's serialized.

For the TaskEditor part: It would be nice if there were a more generic solution for letting the editor update its text when external sources modify it. This is also the root cause of bug #504874 . (Maybe this should get its own bug)

[Lionel Dricot (ploum-deactivatedaccount)]

I did a whole refactoring of the TagStore and solved the freeze here :

https://code.edge.launchpad.net/~gtg/gtg/gtg-tag-rename

There's still one big problem : the TagTree is not refreshed (and it could lead to some bad crashes as it refers to a non-existing tag).

Could someone have a look at this issue ? (simply rename a tag : you will see that the tag is not renamed in the tree as long as you don't open a task related to that tag)

[Lionel Dricot (ploum-deactivatedaccount)]

I've added a manual refresh but it does not help at all : the sort function still have references to the old deleted tag (crashes) and the line is replaced by a blank one (while the new tag is not added).

How could we refresh the treeview each time the Tree() is modified ? I'm missing something here.

[Bertrand Rousseau (bertrand-rousseau)]

I've added some lines in the TagTreeModel to take those modification into account. Now the tree is refreshed correctly. There's still some problems, though:

 - tasks that have an open editor are not refreshed
 - tasks that have no open editor are refreshed when you open their editor, but
 - tasks that had no open editor and have not been opened keep the old tags after closing and restarting gtg

There seems to be some issues with tasks content and metadata refresh: only task opened after tag rename are saved with the new values.

[Lionel Dricot (ploum-deactivatedaccount)]

Fixed in rev. 381