TCP socket semaphores changed to binary sempahore

[deepikabhavnani]

Description

Data from UART can come in before we are ready for wait in receive call. Hence, semaphore release happens before it gets acquired in current implementation. As release was performed for every UART event max count (1024) was reached and as a result semaphore count error was set from RTX.

Requirement was to have a binary semaphore, allowed to perform release any times. Ignore if count is already 1.

Status

READY

Issue:
#4575

[kjbracey]

Sounds plausible to me. But would EventFlags be a better alternative for semaphores now that CMSIS-RTOS2 is available?

[pan-]

While I highly dislike trapping on error because it changes the API behavior- (instead of returning an error it trap the current thread ...) I also believe it is not acceptable to create untested workaround and special hidden rule to fix an issue in the upper layer of the OS. It just adds to the inconsistency of the platform and in the long term it is yet another special case to maintain. It has a cost and it is not negligible.

If semaphore doesn't fit the use case, switch to an appropriate thread synchronization primitive. Event Flags might be a good candidate.

[geky]

Event flags are a good candidate, but we don't have a c++ api in place yet. @YarivCol has started one #4517 but it's not ready for merge yet.

Even with osEventFlags, the regression we're seeing here and in other modules is alarming. I created a general pr that restores the original behaviour here #4579, if that is merged we can also take in the *Socket.cpp changes in @deepikabhavnani's patch to remove the extra cycles spining on the semaphore.