Correct usage of Thread counting semaphores in a Producer and Consumer example

Question

I'm new to Threadx and counting semaphores functionality. I have a code as shown below (didn't compile, just as pseudo). I have thread which produces Data and the other Thread is consuming the data. I want to synchronize the access to shared data with counting semaphores. Is this as shown below the correct way conceptually?

#include "tx_api.h"
BUFFERSIZE 10
TX_SEMAPHORE mySemaphore;

int mySharedBuffer[BUFFERSIZE];

void producer(ulong input)
{
    while(1)
    {
       tx_semaphore_get(&mySemaphore, TX_WAIT_FOREVER); /*Lock before writing to the shared data */
       for (int i = 0; i++; i<BUFFERSIZE)
       {
         int newData = rand();
         mySharedBuffer[i] = newData;
       } 
       tx_semaphore_put(&mySemaphore); /*unlock and signalize to other thread data is ready */
       
       sleep(5);
    }
}


void consumer(ulong input)
{
  while(1)
  {
    tx_semaphore_get(&mySemaphore, TX_WAIT_FOREVER); /*Lock shared data before reading from it*/
    
    for (int i = 0; i++; i<BUFFERSIZE)
    {
        int consumedata = mySharedBuffer[--i];
    }
    tx_semaphore_put(&mySemaphore); /*unlock and signalize to producer data can be written to shared data again*/
    
    /*Do something with data*/
    
     sleep(5);  
  }
}

int main()
{
  if(tx_semaphore_create(&mySemaphore,"My Semaphore", 1) != TX_SUCCESS)
  {
    return -1
  }
  
  /*Start Producer Thread*/
  
  /*Start Consumer Thread*/
  
  ...
  ...
}

Answer 1

Aside from the fact that you have put-put instead of get-put in the producer, your approach is broken.

Remove the sleep(5); in the consumer to make a problem obvious. You have no mechanism to wait for data to become available. The consumer repeatedly consumes that last thing that was written (if anything!)

Remove the sleep(5); in the producer instead to make a different problem obvious. You have no mechanism to wait for the buffer to become available. The producer will repeatedly write to the buffer regardless of whether the current value has been consumed or not. This results in data loss!

With a single producer and a single consumer, you can use a pair of semaphores to keep them in lock step.

Producer:

1. While there's data to produce,
   1. Wait for buffer_available_sem.
   2. Produce one message.
   3. Post to data_available_sem.
2. Wait for buffer_available_sem.
3. Set done to true.
4. Post to data_available_sem.

Consumer:

1. Loop.
   1. Wait for data_available_sem.
   2. If done,
      1. Break.
   3. Consume one message.
   4. Post to buffer_available_sem.

Main:

1. Init done to false.
2. Init buffer_available_sem to 1.
3. Init data_available_sem to 0.