Protecting Shared data and sharing same stack

Question

How Do I protect the shared resources? Need to identify which lines of code use the shared resources and protect them.My guess is that the pop and push resources are shared. So to protect them would I put those functions under a protect label: like there is private: and public:? Also how to I make the 200 threads that I created share the same stack. Update: my professor said that top is a shared resource.

/*
* Stack containing race conditions
*/
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

// Linked list node
typedef int value_t;
typedef struct Node
{
  value_t data;
  struct Node *next;
} StackNode;



// Stack function declarations
void push(value_t v, StackNode **top);
value_t pop(StackNode **top);
int is_empty(StackNode *top);

pthread_mutex_t mutex; 

//--Tom  This is the wrong function signature for thread entry functions
void *testStack(void *arg)
{
  StackNode *top = NULL;
  for (int i = 0; i < 500; i++)
  {

     pthread_mutex_lock(&mutex);

    // --Tom  Mix these up a bit more
    push(5, &top);
    pop(&top);
    push(6, &top);
    pop(&top);
    push(15, &top);


    pop(&top);
    pthread_mutex_unlock(&mutex);

  }
  pthread_exit(0);
}




int main(int argc, char *argv[])
{
  //--Tom   defining mutex on the stack is not a good choice.  Threads don't share data on the stack

  pthread_mutex_init(&mutex, NULL);




  for (int i = 0; i < 200; i++)
  {
    pthread_t tid;
    pthread_attr_t attr;

    pthread_attr_init(&attr);

    //--Tom this is the wrong place to lock.  Need something that sourounds only the code accessing shared resources

    //--Tom argv[1] in not what yo want to pass the thread
    pthread_create(&tid, &attr, testStack, NULL);

    //--Tom  You are not allowingthe threads to run in parallel


  }

  return 0;
}

// Stack function definitions
void push(value_t v, StackNode **top)
{
  //--Tom  you have not identified the critical lines of code and protected them
  StackNode *new_node = malloc(sizeof(StackNode));

  new_node->data = v;
  new_node->next = *top;
  *top = new_node;
}

value_t pop(StackNode **top)
{
  //--Tom  you have not identified the critical lines of code and protected them
  if (is_empty(*top))
    return (value_t)0;

  value_t data = (*top)->data;
  StackNode *temp = *top;
  *top = (*top)->next;

  free(temp);

  return data;
}

int is_empty(StackNode *top)
{
  //--Tom  you have not identified the critical lines of code and protected them
  if (top == NULL)
    return 1;
  else
    return 0;
}

Answer 1

how to I make the 200 threads that I created share the same stack

A first possibility is to have a global variable StackNode *top;, but it is a problem if you want to reuse the same code for different Stacks.

A second is to have that variable local in the main and give its address in parameter when you start a new thread in place of the NULL you currently have, then arg of testStack is in fact a StackNode **

---

Do not manage the mutex before/after to call the push/pop ,manage it inside the functions, else there is a high risk to forget the protection. So the mutext doesn't appear in testStack. In that case warning, see my comment about is_empty

---

int is_empty(StackNode *top)
{
  //--Tom  you have not identified the critical lines of code and protected them
  if (top == NULL)
    return 1;
  else
    return 0;
}

why so complicated ?

int is_empty(StackNode *top)
{
  //--Tom  you have not identified the critical lines of code and protected them
  return (top == NULL);
}

Yes the stack is not modified and it do not look inside, so is not critical only for the empty point of view, so warning :

/* ptop is a StackNode ** */
if (!is_empty(*ptop)) 
  // here the stack can be empty anyway because an other thread got the CPU
  pop(ptop);

If you want to offer a protection region where you can do several operations this must be done using the same mutex (probably hidden by functions regionEnter and regionExit) and because it will also be lock/unlock inside the called function that mutex need to be recursive ( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP )

---

It can be interesting to hide the mutex grouping it and top in an other struct, that allows to not share the mutex for different Stacks