Ticker: program stuck when attaching callback with very small time < 25us

[maciejbocianski]

Program stuck in function Ticker::attach_us when called with very small time < 25us

3  // sample code (1) ticker version
4  Ticker ticker;
5  ticker.attach_us(callback(fun), 15);
6  // program never reach this line !!!

3   // sample code (2) timeout version
4   Timeout timeout;
5   Thread thread;
6  
7   void wait_callback(void)
9   {
10     timeout.attach_us(callback(wait_callback), 15);
11  }
12
13  void run(void)
14  {
15     timeout.attach_us(callback(wait_callback), 15);
16     Thread::wait(10000);
17  }
18  
19  thread.start(callback(run));
19  // program never reach this line !!!

Description

• Type: Bug
• Priority: Major

---

Bug

Target
tested on: K64F|NUCLEO_F070RB|NRF51_DK

Toolchain:
GCC_ARM|ARM|IAR

mbed-os sha:
master

Expected behavior
callback is called
Actual behavior
program stuck or callback is not called
Steps to reproduce
Get change #5006
Modyfy one of the test like below
test_attach_us_time<10> 10us callback
run ticker test e.g. "mbed test -t ARM -m k64f -n tests-mbed_drivers-ticker -vv"

[maciejbocianski]

@bulislaw @0xc0170

[bulislaw]

The callback time is very small and may be too small for clock resolution for given board, but it shouldn't get stuck and documentation should mention this case (and document what will actually happen). @maciejbocianski can you provide a resonable fix?

CC: @c1728p9 What do you think? And maybe it's something we should consider while working on new HAL for tickers?

[c1728p9]

Maybe the interrupt is occurring faster than the mcu can handle and this is preventing threads from running. Do you see the same behavior if you repeatedly schedule a Timeout with the same delay from a thread?

[maciejbocianski]

will check it

[maciejbocianski]

Below code generates same behaviour

3   // sample code 
4   Timeout timeout;
5   Thread thread;
6  
7   void wait_callback(void)
9   {
10     timeout.attach_us(callback(wait_callback), 15);
11  }
12
13  void run(void)
14  {
15     timeout.attach_us(callback(wait_callback), 15);
16     Thread::wait(10000);
17  }
18  
19  thread.start(callback(run));
19  // program never reach this line !!!

[geky]

Hi @maciejbocianski. The Timeout/Ticker classes run their callbacks in interrupt context. That is, the code is executed at a highest priority than the rest of the system since the interrupt is triggered by hardware.

In this situation, the timeout is small enough that when the callback finishes executing, the timeout has already expired and been triggered again. Since the timeout is a hardware timer, it will be counting while the callback is executing, and even returning from the callback may take too long.

Even though you attached the callback in a thread, the callback will be called in interrupt context.

A simple solution is to just keep the timeout in the thread, this will let preemption give other threads a chance to run:

Thread thread;

void run(void)
{
    while (true) {
        wait_us(15); // wait instead of timeout
        wait_callback();
    }
}

thread.start(callback(run));

Note: This may make your timing inaccurate, since it does take time to execute other threads. Unfortunately, your processor only has so much processing time.

[maciejbocianski]

The only solution I can propose is to fix documentation and add some runtime warning to prevent programmer from setting too small intervals

    // mbed-os\drivers\Ticker.h
    /** Attach a function to be called by the Ticker, specifiying the interval in micro-seconds
     *
     *  @param func pointer to the function to be called
     *  @param t the time between calls in micro-seconds
     *
     *  @note setting t to very small number much less than system tick frequency
     *  can cause system malfunction or even system hang
     *
     */
    void attach_us(Callback<void()> func, us_timestamp_t t) {
    	MBED_ASSERT(t);
    	debug_if((t < 1000) ? 1 : 0, "!!! Warning: Scheduling Ticker events with very small time (< 1000us) can cause system hang !!!");
        _function = func;
        setup(t);
    }

What do you think? @bulislaw @c1728p9 @0xc0170

[bulislaw]

CC: @pan-

I'm not sure about the warning. Maybe we can only document it in the function doxygen.

[maciejbocianski]

#5045
@bulislaw

[c1728p9]

You should be able to set the time to Ticker and Timeout to as small as you want and the interrupt should still fire as expected. If the time is small enough then the interrupt will constantly be processed (leaving no time for threads), but should not cause a crash or lockup. As soon as the processing is stopped the thread should resume as normal.

Below is more what I had in mind for the Timeout class. If this locks up then there is a bug in the devices HAL that needs to be fixed.

#include "mbed.h"

Timeout timeout;
volatile uint32_t count;

void wait_callback(void)
{
    count++;
}

int main()
{
    printf("Starting test\r\n");
    for (int i = 0; i < 10000; i++) {
        uint32_t local_count = count;
        timeout.attach_us(callback(wait_callback), 1);
        while (local_count == count);
    }
    printf("No lockup or dropped interrupt\r\n");
    return 0;
}

[maciejbocianski]

@c1728p9
You are right this code should work since we have finite number of interrupts to process.

I have tested your code on NRF51_DK/NUCLEO_F070RB/K64F/NUCLEO_F429ZI and works fine except NUCLEO_F070RB which requires at least 13us to pass this case.

But in case of Ticker if we set small enough timeout then only calback will be scheduled and we will not have chance to cancel it. The only way to cancel it is to put ticker.detach() inside callback.

Finally I agree this is not a bug.

[LMESTM]

I have tested your code on NRF51_DK/NUCLEO_F070RB/K64F/NUCLEO_F429ZI and works fine except NUCLEO_F070RB which requires at least 13us to pass this case.

Test on NUCLEO_F070RB should work fine as well with fix from #5012 - this was just merged

[maciejbocianski]

@LMESTM
Tested with fix #5012 and it work fine for NUCLEO_F070RB with 1us timout

[0xc0170]

Finally I agree this is not a bug.

@maciejbocianski What is the status of this bug report?

[maciejbocianski]

not a bug
comment updated in #5045