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

RTOS/RTX: Prevent to release more semaphore than the number of resource #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oliviermartin
Copy link

Prior to this change, calling osSemaphoreRelease() too many time
will allocate additional resource to the initial number of resources.

Prior to this change, calling osSemaphoreRelease() too many time
will allocate additional resource to the initial number of resources.
@ombre5733
Copy link

I think that limiting the maximum number of tokens to the initial value is no good idea. There are scenarios where it is possible that the semaphore count increases past its initial value.

Take a producer-consumer setup as an example. Whenever the producer finishes its work, it puts the result into a queue and increases a semaphore with osSemaphoreRelease(). The consumer blocks on this semaphore with osSemaphoreWait() and takes an element from the queue when it gets a token. Initially, there are no elements in the queue, of course, meaning that the semaphore has to be initialized with zero. So the producer must be able to increase the token count past its initial value.

In my opinion, calling the function osSemaphoreRelease() is a quite unfortunate because mentally one can only release what has been acquired beforehand. However, this is the only function to increase the semaphore value. Something like osSemaphoreIncrease() (probably paired with osSemaphoreWaitAndDecrease()) would make more sense (especially for the producer-consumer example). Or maybe osSemaphorePut() and osSemaphoreGet()?

So I think, this patch was much more useful, if max_token was not the same as the initial token count. For the producer-consumer example, I'd like to start out with zero tokens and set the maximum number of tokens to the size of the queue.

@ombre5733
Copy link

One more thing: When introducing an upper limit for the semaphore, one should also re-think the behavior of osSemaphoreRelease() when calling it on a semaphore that has already the maximum number of tokens in it. In this case, it could be helpful, if the call was blocking (with a timeout).

@ilg-ul
Copy link

ilg-ul commented Jan 6, 2016

@ombre5733 is right, max_token should not be initialised with the initial count for many reasons, one of them being that it might be 0.

If you really need a max_count, it should be passed as a separate value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants