Managing dynamic array in multi-threaded C program

[champnim]

I am making a multi-threaded C program which involves the sharing of a global dynamic integer array between two threads. One thread will keep adding elements to it & the other will independently scan the array & free the scanned elements. I will use realloc to increment the array & use a local int pointer in the other thread to keep track of the elements scanned. However, my problem is how to free the elements after they have been scanned by my 2nd thread. I tried by having a pointer point to each element of the global array in a for loop & then using the free function, but it only cleared the 1st element. Can somebody please suggest a way out.
On a side note, is this the right way to handle this problem?

[GNiewerth]

Are you bound to C or can you use C++?

[champnim]

I have to do this using C.

[Paul McKenzie]

I tried by having a pointer point to each element of the global array in a for loop & then using the free function, but it only cleared the 1st element. Can somebody please suggest a way out.

Please post code. There are too many unknowns in your description to properly answer your question.

Regards,

Paul McKenzie

[champnim]

int *arr; //global variable

//This is a part of a thread for reading input
int count=0;
while(1)
{
    //Input has been read...set value of int i
    count++;
    arr = (int*) realloc (arr, count * sizeof(int));
    if (arr==NULL)
       { puts ("Error (re)allocating memory"); exit (1); }
    arr[count-1]=i;       //Value of i depends on input read
}

//This is a part of the main thread
int j=0;
int *input;
while(1)
{
         input=arr[j];
         if(input==i)
         {
                //perform operations
                j++;
                free(input);
         }
         else
               //do something else
}

Here what I require is that the 1st thread will read input & based on it store values of 'i' in the global array 'arr'. Independently of that, the 2nd thread will start accessing the elements of 'arr' from the start. After every value of 'arr' read, 'input' should free that memory so that the memory usage remains restricted. The increment in 'j' will ensure that the main thread accesses the values stored in 'arr' in the correct order. This is what I am looking to do. If anybody can point out the mistake(s) in the above code, I would be highly obliged. If there is any alternative method that would serve my purpose better, please suggest it.

[GNiewerth]

You´re gonna need some kind of access synchronization, probably a critical section. Second I recommend the use of a linked list instead of reallocating the array each time you append or remove an element. The following pseudocode should make things clear:

// commonly accessible critical section
CriticalSection Sync;

// commonly accessible LinkedList
List FIFO;

// Thread code
void run()
{
   while( true )
   {
      // Enter critical section
      Sync.enter();

     // append item to list
     FIFO.push( NewData );

     // Leave critical section
     Sync.leave();

     // wait 100 msec
     sleep( 100 );
   }
}

main thread
int main()
{
   while( true )
   {
       int iValue;
       bool bHandle = false;

       // enter critical section
       Sync.enter();
       
       // assume there´s no item to handle
       bHandle = false;

       if( false == FIFO.empty() )
       {
          // get first item from FIFO
          iValue = FIFO.top();

          if( iValue == someValue )
          {
              // remove front item from FIFO
              FIFO.pop();

             // indicate there´s an item to handle
             bHandle = true;
          }
      }
      // leave critical section
      Sync.leave();

      if( true == bHandle )
      {
          // handle item
          handle_item( iValue );
      }
      // sleep some time
      sleep( 100 );
   }
}

The critical section ensures that there´s only one thread modifying the FIFO at any given time to prevent race conditions.
The worker thread just waits until it can enter the critical section and then appends a new item to the FIFO and leaves the critical section, allowing the main thread to access the FIFO.
The main thread waits until it can enter the critical section and then looks at the front item (if existent) of the FIFO. When it has to handle the item it removes it from the FIFO and sets a flag to true.
The tricky part is to divide the FIFO handling and the item handling into two separate steps, the solution presented blocks the critical section only for adding a new or removing the front element, time consuming item handling occurs in an uncoupled second step.

[Lindley]

//This is a part of the main thread
int j=0;
int *input;
while(1)
{
         input=arr[j];
         if(input==i)
         {
                //perform operations
                j++;
                free(input);
         }
         else
               //do something else
}

What do you think you're doing here? You can't free a single element of an array. I'm surprised your program hasn't just crashed. You can only free an entire array at once.

Using a linked list is really a much better option if you want to remove elements from the front.

[Paul McKenzie]

int *arr; //global variable

//This is a part of a thread for reading input
int count=0;
while(1)
{
    //Input has been read...set value of int i
    count++;
    arr = (int*) realloc (arr, count * sizeof(int));
//...
}

//This is a part of the main thread
int j=0;
int *input;
while(1)
{
         input=arr[j];
         if(input==i)
         {
                //perform operations
                j++;
                free(input);
//...

The only thing you can "free" is the pointer returned to you when you called realloc(). This is the case regardless of if you're using threads or not. Your code from the surface cannot guarantee that you're calling free() on the value returned by realloc().

That's why it was important to post your code, since your problem of not freeing all the space seems to be a misunderstanding of basic handling of dynamically allocated memory, and not an issue with threads.

As Lindley pointed out, you should use a linked list. But to add to that, if you are going to use a linked list, code the linked list first, independent of the program you're writing now. Then test it. Then when that's done, add any synchronization needed. Then apply it to your current app.

Regards,

Paul McKenzie

[champnim]

Sorry I'm back people...Well I followed your suggestions & switched from an array to a linked list. The linked list is working fine independently. however with the program, it is giving problems at times. Even more problematic is the combination of the linked list with the criticalsection thing. Here's the code:

struct fifo
{
    int result;
    char *info;
    struct fifo *next;
}*node;

void add(struct fifo **n,int data,char *text)
{
    struct fifo *temp,*node1;
    if (*n==NULL)
    {
        temp=(struct fifo *)malloc(sizeof(struct fifo));
        printf("%d is to be stored\n",data);
        temp->result=data;
        printf("%d is stored\n",temp->result);
        temp->info=(char *)malloc(strlen(text)+1);
        strcpy(temp->info,text);
        temp->next=NULL;
        *n=temp;
    }
    else
    {
        temp=*n;
        while(temp->next!=NULL)
            temp=temp->next;
        node1=(struct fifo *)malloc(sizeof(struct fifo));
        node1->result=data;
        node1->info=(char *)malloc(strlen(text)+1);
        strcpy(node1->info,text);
        node1->next=NULL;
        temp->next=node1;
    }
}

void del(struct fifo **n)
{
    struct fifo *temp;
    if (*n==NULL)
    {
        printf("Linked list is empty\n");
    }
    else
    {
        temp=*n;
        *n=temp->next;
    }
}

//Inside read thread that will add to the linked list
            //If a particular condition is satisfied
               EnterCriticalSection(&critical);
               add(&node,50,location);                //location holds the string read
               LeaveCriticalSection(&critical);       //exit condition after this

//Inside main thread that reads & deletes from the linked list
InitializeCriticalSection(&critical);

while(1)
{    
     if (node==NULL)
    {
        printf("Nothing to read\n");
        Sleep(4000);
    }
    else
    {
        EnterCriticalSection(&critical);
        temp=(struct fifo *)malloc(sizeof(struct fifo));
        printf("%d\n%s\n",node->result,node->info);
        op=node->result;
        printf("op is %d\n",op);
        detail=(char *) malloc (strlen(node->info));
        strcpy(detail,node->info);
        printf("%s\n",detail);
        del(&node);
        LeaveCriticalSection(&critical);
        if (node==NULL)
            printf("node is null\n");
        else
            printf("node is not null\n");
        if (op==50)                                 
        {
            //call function with detail as parameter...call is successful
            free(detail);
            Sleep(2000);
            //Make some other function calls
            op=-1;
            if (node==NULL)
                    printf("%d\n",op);
            else
                    printf("node is not null\n");
            printf("%d\n",op);
        }
        //some other similar conditions 
}

DeleteCriticalSection(&critical);
//Close threads
return (0);
//End of main

Now, there are no errors in compilation. When I read something, main thread reads that & deletes the node. I get a confirmation of the same due to the printf given after the del(&node). Then, i go into my function calls, which give the right result. But at the end of it I get an error & my printf statement shows that on the 2nd check node is not null.
To make things clear this is how the output looks like:

Nothing to read
//after some time
50         //value of int stored in node
25         //string stored in node
op is 50
25
node is null
//results of function calls successfully printed
node is not null
-1

Please help me find the mistake...I'm at my wit's ends.

[champnim]

Hey guyz I have managed to narrow down my problem. The read thread is reading input from a COM port. Now when it has to read a very large input, it does so in multiple read operations. The error that I am getting is in such a situation. When the thread had to read only a small string(one that could be read in a single read), I got the confirmation that the node is indeed null. But in case of a multiple read, it kept showing "node is not null".
Can anybody please tell why this is happening & how I should deal with this.