xtimer: broken on 'native` when with thread_flags

[haukepetersen]

refining #6624

The following code hangs after the first timeout on native:

static void time_evt(void *arg)
{
    thread_flags_set((thread_t *)arg, 0x1);
}

int main(void)
{
    xtimer_t timer;
    timer.callback = time_evt;
    timer.arg = (void *)sched_active_thread;

    while (1) {
        xtimer_set(&timer, 1000000);
        thread_flags_wait_any(0x1);
        printf("+++ timeout +++\n");
    }

    return 0;
}

After a quick debugging session I might have found a lead: it seems like native is doing an early context switch from its interrupt context back to the main thread when calling thread_flags_set(). The second call to xtimer_set() exits in _lltimer_set() without setting the new timer, because _in_handler is still set. If I see it correctly, the _in_handler being set because the _timer_callback() function never actually exists -> hence it seems like the switch from interrupt to thread context is happening early.

I am not sure however, how this should be fixed?! Are there maybe some if (in_isr()) checks missing in the thread_flags implementation?

[haukepetersen]

The problem seems to be broken scheduling behavior in native, isr_thread_yield() (in native_cpu.c) calls the scheduler directly, and does not allow the interrupt to run to completion. If I am wrong or is that quite broken behavior?!

[jnohlgard]

@haukepetersen I agree that the ISRs must be allowed to run to finish before switching contexts.

[lebrush]

I faced a similar situation when implementing the xtimer_mutex_lock_timeout and solved by adding a thread_yield_higher() (which then I figured out it should rather be a sched_context_switch_request = 1 since it's called from an interrupt context) in the callback.

Maybe that was then a native pitfall after all but IIRC @OlegHahm saw the same behaviour in a SAM platform.

EDIT: found the #6441 (comment). The board was a samr21-xpro I guess adding the sched_context_switch_request = 1 in the callback fixed that too.

[haukepetersen]

ISRs must be allowed to run to finish before switching contexts.

Agree, this is not only a should, but a MUST!

So it seems this behavior is (to some extend) related to #6442. The question is how we can fix this once and for all... IMHO this should be a high priority thing to do, but I don't have the know-how to fix this. Anyone else volunteering?

[OlegHahm]

Maybe I don't it right, but why do you assume that the ISR does not run to completion? I can't neither see it from the native code (just calling sched_run()) does not immediately switch context) nor from putting a debug puts() after thread_yield_higher() in _thread_flags_wait().

[haukepetersen]

I'll put together a test application to demonstrate this behavior.

[haukepetersen]

done: #6659

[kaspar030]

#6660 fixes the issue for me.