search

nasa / osal

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

Remove OS_BinSemFlush #884

Open skliper opened 3 years ago

skliper commented 3 years ago

Is your feature request related to a problem? Please describe. Leads to race conditions, shouldn't be used. Also other flushes aren't implemented, so consistency. https://github.com/nasa/osal/blob/ead5723f59ee49b36a36d6cb66c4cfe680ebbd68/src/os/vxworks/src/os-impl-binsem.c#L145

Describe the solution you'd like Deprecate/remove

Describe alternatives you've considered Leave as-is (future work)

Additional context None

Requester Info Jacob Hageman - NASA/GSFC, OSAL code review

jphickey commented 3 years ago

I would actually recommend postponing this removal to a future version. There's a fair bit of logic in the POSIX bin sem implementation to allow/support this API and removing it would then mean it should all be scrubbed out too. Basically this makes the change bigger than it seems.

My recommendation would be to simply document that the OS_BinSemFlush suffers from a race condition if/when combined with a conditional test, and that users are advised not to rely on this function for this reason.

At this late stage of the release cycle it is just safer to leave it alone for now. Plus, in VxWorks 7 maybe we can close the race condition in some other way.

zanzaben commented 3 years ago

After I started looking at it I also realised that it would be a much larger change then initially seemed and also agree to postpone doing it.

skliper commented 9 months ago

@jphickey - This is specific to VxWorks 6.9 right?