sys: xtimer: fix some race conditions

[kaspar030]

This PR (hopefully) fixes two issues:
#4902:

previously, _xtimer_set_absolute() would determine the current _long_cnt outside of a critical section, then determine using _this_high_period() whether the timer shoots in current or next period. If the overflow interrupt occurs in between, long_target is too low (and thus not in _this_high_period()), causing the timer to wrongly be added to the long list. This PR moves the critical section over initial _long_cnt determination.
#4841:

Previously, xtimer_set() would remove a timer outside of a critical section, then call _xtimer_set_absolute(). If the same timer would be set in between, it could end up twice in a timer list, causing xtimer to hang.
This PR refactors xtimer_remove into a safe and unsafe variant and adds a second check using the unsafe variant in _xtimer_set_absolute()'s critical section.

[kaspar030]

@immesys Could you give this a try?

[immesys]

@kaspar030 sure. I will run it overnight and let you know.

[kaspar030]

There's one thing this PR doesn't fix: the same race as @immesys, for spinning timers.

I didn't know what would be the best semantic:

1. after spinning, shoot the currently set callback of the timer, which might have been overridden on timer re-set (current state, probably not the expected behaviour)
2. check at every spin cycle if the timer has been reset (inefficient)
3. copy the timer object, use that (different semantics than without spinning)
4. check at the end of spinning if the timer has been reset, possibly aborting (might spin and then do nothing)

I tend to go with 4.), as it is easy to implement and probably leads most to expected behaviour.

[kaspar030]

Thanks @immesys!

[jnohlgard]

@immesys @kaspar030 what's the status here?

Needs rebase btw

[immesys]

About to do some more testing of this, but @kaspar030 it needs some rebasing.

[kaspar030]

• rebased

[immesys]

Sweet!

[jnohlgard]

ACK for the change in general, waiting for @immesys report from their testing

[immesys]

I actually hit some timer problems today, but as I was hacking on something else, it might have been an unrelated newly introduced user error. Can you give me tomorrow to test it specifically? Ideally I might also be able to contribute a test case.

[kaspar030]

@immesys All the time you need! ;)

[miri64]

Any progress?

[immesys]

Huh, I commented on this, maybe it was on the issue not the PR. I think this looks good and should be merged. If I track down why I was still getting a lockup in _remove then I will open a new issue but it is very rare and might be the debugger interfering with things.

[miri64]

👍 then go.

[miri64]

(sorry just saw that this passed and was ACKed. Did not track the issues to that)