unit tests of callback_timer_locked - inconsistence with timer design

[KammutierSpule]

test_callback_timer_locked.cpp

It tests timer in a way that is not possible to be used.

    void callback2()
    {
      tick_list.push_back(ticks);

      p_controller->start(2);
      p_controller->start(1);
    }

callback2 calls start() (which has lock/unlock), while callback2 itself is called on a tick() (which is lock too).
This would call a deadlock.

Relates to #952

[KammutierSpule]

I'm wondering if it was considered that a recursive_mutex could be used (this way it won't cause a deadlock), but, I'm not sure how the active_list would work
while (has_active && (count >= active_list.front().delta))
if some time is started during the tick(), then on the tick() loop it is keep removing the count time from active_list?
(I couldn't clarify this by looking into active_list code)

[jwellbelove]

I could possibly add member functions that can be used when the execution is within the timer's callback.

i.e. start_from_callback()

These would not check or set the locks. I use something similar in etl::queue_spsc_isr where certain functions are allowed to be called from within an ISR.

[KammutierSpule]

I could possibly add member functions that can be used when the execution is within the timer's callback.

i.e. start_from_callback()

My main wondering is this design option:

while (has_active && (count >= active_list.front().delta))
if some time is started during the tick(), then on the tick() loop it is keep removing the count time from active_list?

because, event if is ok to insert, what happens to this loop with things that are inserted in the middle?
I'm wondering if I understood the algorithm correctly, I think if you add something in the middle of that loop, it will remove time to the delta when it shouldn't (but only should do it on the next tick(), not on that same one)

[KammutierSpule]

Example: consider there is only 1 single shot time and it will expire. In the callback it calls start (insert) time:

            while (has_active && (count >= active_list.front().delta)) // first time it enters here, but it will get here on next loop too
            {
              timer_data& timer = active_list.front();

              count -= timer.delta;

              active_list.remove(timer.id, true); // time is removed here

              if (timer.callback.is_valid())
              {
                timer.callback(); // if a time is inserted here, it will be added to the queue front and the loop removes (incorrectly).
              }

              has_active = !active_list.empty(); // if a timer was added on callback, list is not empty
            }

as per this ticket, it is ok if it is assumed it works this way by design (i,e: it is not possible to call timers functions on callback), but then, the unit tests are not ok with this design.

[KammutierSpule]

On the unit tests, timeout of timers:

id1 = 100 (it will trigger id2 and id3 start)
id2 = 10
id3 = 22

ticks id2 id3
102 10 22
105  7 19
108  4 16
111  1 13
114    10 <-- id2 should expire here, but on test it test against 111
117     7
120     4
123     1
126       <-- id3 should expire here, but on test it test against 123

From test:
std::vector<uint64_t> compare1 = { 102, 111, 123 };

id2 with a timeout of 10, 111-102 = 9 so it couldn't timeout yet.

[KammutierSpule]

while (has_active && (count >= active_list.front().delta))

I'm not sure if this would work (with your list) but if this code could be made to start in one front node and only go backwards (leave any eventual new entry on the front) then it would allow new entries and won't remove the delta on new ones.

Something like:
`
current last node = active_list.front();
while (has_active && (count >= current last node.delta)) {
current node = current last node;

// only get the last to the beginning from the initial node:
if (current last node.previous) &&
(count >= current last node.previous.delta) {
current last node = current last node.previous
}
...
// do regular stuff..
...
}
`

this only works if using recursive mutex.