Issue with the task handle

[Serotonindude]

Hi,
we've encountered a problem with the task handle which is passed to the user when a task is started. Sometimes the Timer class just wouldn't remove the task on cancellation. It just didn't pass the test

if (t->handler && (t->id ^ task) == (uintptr_t)t)
            {
                remove(t);
                break;
            }

in some cases. Of course we made sure our task handles were correct and not already cancelled etc. The automatic removing when the handler returns false worked fine, which does not do this test.
We've then removed the obscuring of the actual address of the internal task struct via its id from this test and also from the task_id method in a local copy of the header file:

cancel(Task &task)
    {
        if (!task)
            return;

        timer_foreach_task(t)
        {

            // the author obscured the true address of the internal task struct by XORing the address with the id of the internal task struct
            // actually, this should work, but it doesn't... so we just removed the obscuring :/
            //if (t->handler && (t->id ^ task) == (uintptr_t)t) {
            if (t->handler && (task == (uintptr_t)t))
            {
                remove(t);
                break;
            }
        }

        task = (Task)NULL;
    }

inline Task
task_id(const struct task *const t)
{
        // the author obscured the true address of the internal task struct by XORing the address with the id of the internal task struct
        // actually, this should work, but it doesn't... so we just removed the obscuring :/
        // const Task id = (Task)t;
        //return id ? id ^ t->id : id;

        return (Task)t;
}

This solved the problem. I know, it should actually work, as A ^ B ^ B = A but for whichever reason (compiler, pointer stuff, whatever...) it didn't...

So is there any deeper need for the address obscuring? If not, maybe it could be removed?

Kind regards,
Sero

[contrem]

So is there any deeper need for the address obscuring?

There are two reasons it's implemented this way:

1. To prevent task ids from one timer canceling a task from another timer - programmer error
2. To prevent a stale task id from canceling the wrong task - programmer error, but much harder to identify

Returning the address as suggested only provides benefit 1, but not 2, and will fail in a case like this:

tid_1 = timer.at(1, handler);
//  1 milli passes and handler gets executed
tid_2 = timer.at(100, other_handler)
timer.cancel(tid_1); // ERROR - this might cancel other_handler since tid_1 == tid_2 is possible

It's easy to see that tid_1 handler already finished and doesn't need to be canceled in this linear example, but it might not be so clear in more realistic use cases.

If you could provide example code showing the error, maybe we can help diagnose the issue.

[Serotonindude]

Dear contrem,
thank you for the fast and useful reply.
After checking that we don't use a second timer to create the tasks and also making sure, that task* and int have the same size in memory, we checked the state of the task array at various points in time and found that it's memory got corrupted. We managed to narrow the error down to an out of bounce array access in another part of the program. After fixing this, everything works fine again with the timers... So false alarm. Sorry for that. :)
Thanks for the help!
Kind regards,
Sero