xxyzz / ostep-hw

Operating Systems: Three Easy Pieces(OSTEP) homework and project solutions
GNU General Public License v3.0
763 stars 179 forks source link

29/hand-over-hand-locking-list.c has data race when inserting node #8

Closed MarekZhang closed 3 years ago

MarekZhang commented 3 years ago

Hi there, First, I want to thank you for this amazing repo which helps me a lot.

static void List_Insert(list_t *L, int key) {
  // synchronization not needed
  node_t *new = malloc(sizeof(node_t));
  if (new == NULL)
    handle_error_en(errno, "malloc");
  new->key = key;
  Pthread_mutex_init(&new->lock, NULL);

  // just lock critical section
  pthread_mutex_t *tempLock = &L->head->lock;
  Pthread_mutex_lock(tempLock);
  new->next = L->head;
  L->head = new;
  Pthread_mutex_unlock(tempLock);
}

This snippet of code does not eliminate the race condition when there are more than two threads inserting nodes to the list. Imagine thread_1 and thread_2 try to lock the head of the list and only one thread succeded. The other thread is blocked but holding the same head node as the other thread. After the thread(which successfully locked the head node of the list) inserting a new node into the list, what the blocked thread should do is locking the new head. But it will insert a node as the predecessor of the old head node.

I think we still need a global lock for inserting nodes into the list.

xxyzz commented 3 years ago

Thanks for pointing it out! Feel free to reopen this issue if there are still some bugs.