search

nasa / osal

The Core Flight System (cFS) Operating System Abstraction Layer (OSAL)
Apache License 2.0
546 stars 213 forks source link

RTEMS `OS_BinSemTimedWait_Impl` and `OS_CountSemTimedWait_Impl` return incorrect status #1432

Closed skliper closed 8 months ago

skliper commented 9 months ago

Describe the bug RTEMS_UNSATISFIED is returned with flushed, or when called with RTEMS_NO_WAIT and semaphore isn't available.

Current RTEMS implementation doesn't return the expected OS status for all cases (or match Linux/VxWorks behavior).

Expected handling:

Broken:

To Reproduce Started investigating when CF reported the timed wait OS_DEBUG message on RTEMS when a semaphore wasn't available, the rest was inspection.

Expected behavior Return OS_SEM_TIMEOUT when status is RTEMS_UNSATISFIED with RTEMS_NO_WAIT. https://github.com/nasa/osal/blob/edede38d8cf629fa4490b65dc674ad17b11b1050/src/os/rtems/src/os-impl-binsem.c#L252-L266 https://github.com/nasa/osal/blob/edede38d8cf629fa4490b65dc674ad17b11b1050/src/os/rtems/src/os-impl-countsem.c#L217-L230

Code snips See above.

System observed on:

Additional context Should add functional tests for this and confirm Linux/VxWorks/RTEMS behavior is consistent.

Reporter Info Jacob Hageman - NASA/GSFC

skliper commented 9 months ago

I'll add functional test cases (including #1411).

Note the current bin sem tests and count sem tests are very old and miss a few things (flush of a binary sem timed wait, any counting sem flush tests, all polling tests). Considering rewrite.

New logic under consideration:

  1. Initialize sem w 0 and nonzero, confirm info
  2. poll on 0 should return OS_SEM_TIMEOUT
  3. Validate info
  4. poll on nonzero should return OS_SUCCESS
  5. Validate info
  6. timed wait should return OS_SEM_TIMEOUT
  7. Validate info
  8. Start child task w/ wait, confirm waiting, delete task, check info to confirm sem is still valid (this replicates the old test)
  9. Start child task w/ takes
  10. Confirm waiting
  11. Give and confirm receipt loop until complete
  12. Give prior to next task start, confirm info
  13. Start child task w/ timed wait
  14. Confirm one processed and waits
  15. Give and confirm receipt loop until complete
  16. Start 3 child tasks (2 takes and 1 timed wait)
  17. Confirm waiting
  18. Give a couple times (not enough to exit) and confirm correct state
  19. Flush and confirm all advance 1 cycle
  20. Give to completion and confirm
  21. Valid deletes
  22. Confirm invalid deletes
skliper commented 9 months ago

Note RTEMS OS_BinSemGetInfo_Impl and OS_CountSemGetInfo_Impl should return OS_ERR_NOT_IMPLEMENTED or could lead to unexpected results: https://github.com/nasa/osal/blob/1a8823f2dc93da2bcb3696c92c7aee4f512ed4f7/src/os/rtems/src/os-impl-binsem.c#L275-L279 https://github.com/nasa/osal/blob/1a8823f2dc93da2bcb3696c92c7aee4f512ed4f7/src/os/rtems/src/os-impl-countsem.c#L239-L243

skliper commented 9 months ago

Note - updated title and text since there is no flush API for a counting semaphore.