Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear pending signal when configuring timer #336

Merged
merged 1 commit into from
Jan 8, 2020

Conversation

jphickey
Copy link
Contributor

Describe the contribution

Fix #335

On POSIX, call sigtimedwait() on the selected RT signal as part of the set up for the timebase. This ensures that if the signal is already pending, it will be cleared.

This also simplifies the timer callback routine in the UT code, cleaning up some unnecessary extra logic.

Testing performed
Build with ENABLE_UNIT_TESTS=TRUE and execute osal_timer_UT in a repeated loop.

Prior to this fix, it would typically fail after about 10-20 iterations, sometimes fewer.
After this fix, it does not fail (100+ iterations, no failures).

Confirmed normal operation of CFE core (no change).

Expected behavior changes
Fixes occasional failures in the nominal timer test.
No change to FSW code

System(s) tested on:
Ubuntu 18.04 LTS, 64 Bit

Additional context
Tracked down the root cause of the occasional timer failures to the timer test that preceded it. The previous test involved a short interval (5us). Depending on the timing of the preceding delete call, the timer signal might fire again before the timer is fully un-configured (i.e. during the delete process).

In this case, the system will be left with the signal still pending (blocked) but with no task running to accept/clear it.

This is OK until the next timer is configured, and the same signal will be selected (the first RT signal). In this case, because the signal was already pending from the previous config, it results in an extra "spurious" initial callback at the start. This in turn was interfering with the difference calculations in the timer UT.

It is unlikely that this issue would be seen in FSW code, as it depends on deleting and recreating timers and FSW generally does not do this (i.e. it sets up timers once).

The main part of the fix is to call sigtimedwait() with a zero timeout to ensure that the signal is not pending.

Additionally, there is some cleanup in the UT callback code, which fixes two other possibilities. These were not occurring, but potentially could.

  • The interval computation could be incorrect if the time elapsed was greater than 1 second.
  • Always set the currTime/endTime in case there is a lag time in stopping the timer. But only set "g_status" once.
  • Remove unneeded/invalid "OS_TimerSet" call.

Contributor Info
Joseph Hickey, Vantage Systems, Inc.

Community contributors
You must attach a signed CLA (required for acceptance) or reference one already submitted

On POSIX, call sigtimedwait() on the selected RT signal as part of
the set up for the timebase.  This ensures that if the signal is
already pending, it will be cleared.

This also simplifies the timer callback routine in the UT code,
cleaning up some unncessary extra logic.
@skliper skliper added this to the 5.1.0 milestone Jan 2, 2020
@skliper
Copy link
Contributor

skliper commented Jan 8, 2020

CCB 20190108 - Reviewed and approved for integration candidate

@skliper skliper added the CCB:Approved Indicates code review and approval by community CCB label Jan 8, 2020
@skliper skliper changed the base branch from master to merge-20190108 January 8, 2020 20:47
@skliper skliper merged commit 3bdea3a into nasa:merge-20190108 Jan 8, 2020
skliper added a commit that referenced this pull request Jan 8, 2020
Fix #294 #335
Reviewd and approved at 2020-01-08 CCB
skliper added a commit that referenced this pull request Jan 14, 2020
Fix #335: Clear pending signal when configuring timer
@jphickey jphickey deleted the fix-335-timer-ut-failure branch February 13, 2020 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants