sometimes task is removed before task-deleted signal handler is executed

[Luca Invernizzi]

Signals are executed when "there is time for them", without specific limits.

Now, when we issue the signal task-deleted, we should do a series of things (updating tags in the tag pane, removing the task and its children from the main view...).
A number of times we need to access information about the task that has been removed. Example: we need to check if the tag that we are updating in the tags pane should be shown or not (are there still tasks tagged with that?)

This causes a series of problems, among which the drag and drop "phantom tasks" (tasks that are shown twice), or crashes upon deletion of tasks because the signal arrived too late and we cannot get the list of tags that need to be updated.

How to solve this?instead of passing the tid along with the signal, I think we should pass a copy of the whole task.
I understand there are a series of things that need to be well thought out also with this approach and, since it's late and tomorrow is GTG Regression day, i'll regress to bed.

[Bryce Harrington (bryce)]

On Wed, May 19, 2010 at 01:43:05AM -0000, Luca Invernizzi wrote:
> Signals are executed when "there is time for them", without specific
> limits.
>
> Now, when we issue the signal task-deleted, we should do a series of things (updating tags in the tag pane, removing the task and its children from the main view...).
> A number of times we need to access information about the task that has been removed. Example: we need to check if the tag that we are updating in the tags pane should be shown or not (are there still tasks tagged with that?)
>
> This causes a series of problems, among which the drag and drop "phantom
> tasks" (tasks that are shown twice), or crashes upon deletion of tasks
> because the signal arrived too late and we cannot get the list of tags
> that need to be updated.
>
> How to solve this?instead of passing the tid along with the signal, I think we should pass a copy of the whole task.
> I understand there are a series of things that need to be well thought out also with this approach and, since it's late and tomorrow is GTG Regression day, i'll regress to bed.

Another thought would be to add a Trash Stack; when a delete action
occurs, instead of outright deleting the task, it's moved to the Trash
Stack and a set of flags assigned for the various jobs that have to be
completed. Signal handlers can then reference the task by tid and get
the data they need as they do their stuff.

A reaper process periodically reviews the structure and frees objects
who have all their job flags unchecked.

The Trash Stack would not be persisted across program restarts, so care
will need taken that no valid data references items in it. IOW, make
sure to drop all references to the tid in data structures before putting
it in the Trash.

(This is a pretty crude garbage collection system, there may well be
better pythonic garbage collection systems out there.)

A fringe benefit of this is that an undo/undelete system could be hooked
into it without too much hassle.

[Lionel Dricot (ploum-deactivatedaccount)]

I prefer, by far, Bryce idea to solve the problem. It looks cleaner and, as said, useful for an Undo mechanism.

[Lionel Dricot (ploum-deactivatedaccount)]

Isn't that bug the same as bug #571940 ?

[Luca Invernizzi (invernizzi)]

Yes it is. The trash stack makes perfect sense.

[Luca Invernizzi (invernizzi)]

Possible implementation:
we have a SignalsManager. When you need to intercept a signal you register yourself to the SignalManager. That way, it can keep track of who needs what.

When you receive your signal (either directly or via the signal manager, I think the second one is cleaner), once you've done your business, you tell the SM that you're done. He is a smart guy and can count, therefore he can remove old elements and there no need for garbage collectors.
Am I missing something?

[Lionel Dricot (ploum-deactivatedaccount)]

Luca, could you provide a usecase for this particular bug please ?

[Luca Invernizzi (invernizzi)]

GTG/taskbrowser/tasktree.py

One is this:
delete a task and its subtask at the same time

Traceback (most recent call last):
  File "/home/luca/Projects/gtg/TRUNK/GTG/viewmanager/delete_dialog.py", line 42, in on_delete_confirm
    self.req.delete_task(tid)
  File "/home/luca/Projects/gtg/TRUNK/GTG/core/requester.py", line 208, in delete_task
    for tag in task.get_tags():
AttributeError: 'NoneType' object has no attribute 'get_tags'

Another one is that in tagtree.py, at update_tag_task_deleted() (which updates the tags upon the deletion of a task):
if tasks_count <= 1 sometimes fail because the task has already been deleted.

The problem is that we cannot know what the current state is (task has been already deleted or not?)

[Luca Invernizzi (invernizzi)]

This is caused by misplaced signal emitters.
The signal "task-deleted" was originally intended to be emitted after the deletion (unfortunately, in the code is "almost" after the deletion). The problems with tags is also due to misplaced signals.

Also, in the requester, the task_delete() function has an unfortunate name, since it's not made to delete tasks.
Tasks should be deleted with Task.delete().
That task_delete() function is called at the end of Task.delete() to proceed to the deletion of the task from the backend.

I'm mostly fixing these bugs in my GSoC branch, because I've changed things here and there, so the fixes do not apply directly to Trunk (and writing a fix just for trunk would create a bzr merge hell).

[Luca Invernizzi (invernizzi)]

There was a little confusion in Task about deletion, because most of the times to delete a task "self.req.task_delete()" was called, which was not made for that (see previous comment).
The delete dialog itself called that function, but thanks to some misplaced signals everything worked almost perfectly:D

(in particular, the requester was signaling the deletion and then telling the datastore to delete the task from the backend, which triggered the removal from the tree and finally the removal from the ui. Now it's Task->Tree->UI and Task->TaskSource(aka Backend))

Anyway, revision 800 in the linked branch is solves this.
Turns out that the only place where we needed to access a task after it was deleted was in the Tasktree, since we needed to know the paths to remove. I've added another signal to the filteredtree, "path-deleted", so that we don't have this bug anymore. We do need documentation, though, because I believe all this was caused by a misunderstanding on a function name.

If you didn't understand anything I said, I will repeat tomorrow. Not it's 3:37, the *perfect* time to sleep :)

[Luca Invernizzi (invernizzi)]

Because of liblarch development, this bug isn't relevant anymore.