C многопоточность malloc плохой доступ глобальный - PullRequest
0 голосов
/ 23 февраля 2011

У меня есть 2 структуры следующим образом:

typedef struct _product
{
    unsigned int     product_id;
    time_t           my_time_stamp;
    unsigned int     lifespan;
} product;

typedef struct _queue
{
    product *   elem;
    struct _queue * next;
} queue;

У меня есть глобальная переменная head.

queue *head; 

В основном я malloc глава очереди.

head = (queue *)malloc(sizeof(queue));

И вызовите его в другой функции

    if(head->elem == NULL) { head = newPointer; }

Когда я делаю голову, все в порядке. Затем, когда он переходит к функции, он сбрасывается до 0x0.

Это неправильный способ сделать это? Если так, как это должно быть сделано?

void producer_func(void *tid)
{   
  while(number_of_products_created < number_of_products_max) //check for completeness of task
  {
    my_str("in producer with id ");
    my_int((long)tid);
    br();
    pthread_mutex_lock(&the_mutex);
    my_str("locked by ");
    my_int((long)tid);
    br();
    while(space_in_queue >= size_of_queue)
      pthread_cond_wait(&notFull, &the_mutex); //check to see if there is room in the queue

    product *tempProduct = malloc(sizeof(product));

    //////////////enter critical region/////////////
    tempProduct->product_id = product_id_count++;
    //////////////exit critical region/////////////

    //time
    struct timeval tim;
    gettimeofday(&tim, NULL);
    tempProduct->my_time_stamp = tim.tv_sec; 
    //endtime
    tempProduct->lifespan = rand();
    //new item for queue
    queue *newPointer = malloc(sizeof(queue));
    newPointer->elem = tempProduct;
    newPointer->next = NULL;                

    //critical region//
    if(head == NULL)
    {
      head = newPointer;
    }
    else
    {
      //traverse list
      queue *tempPointer;
      tempPointer = head;
      while(tempPointer->next != NULL)
      {
        tempPointer = tempPointer->next;
      }
      tempPointer->next = newPointer;
    }
    space_in_queue++;
    number_of_products_created++;
    //end critical region//

    my_str("unlocked by ");
    my_int((long)tid);
    br();
    usleep(10000);
    my_str("num products created is ");
    my_int(number_of_products_created);
    br();
    usleep(10000);
    pthread_cond_broadcast(&notEmpty);
    pthread_mutex_unlock(&the_mutex);
    usleep(100000); //let others have a chance
  }
}

Ответы [ 2 ]

4 голосов
/ 23 февраля 2011

Это очень много кода между блокировкой мьютекса и его освобождением.Вы должны стремиться минимизировать объем работы, выполняемой с удерживаемым замком.Вы должны сделать всю работу, кроме добавления элемента в список без блокировки.Только когда все готово, вы захватываете мьютекс и продолжаете добавлять элемент и освобождаете мьютекс.Возможно, было бы неплохо сделать добавление элемента в конце списка операцией O (1), а не O (N) - сохраняйте указатель на хвост структуры.Вы действительно не получите хорошую производительность (параллелизм) из системы, если будете спать 10 миллисекунд за раз, когда мьютекс будет заблокирован.

Другая критика заключается в том, что структура вашего продукта будет занимать 24 байта на многих 64-битных платформах, а не просто 16. Если time_t - это 8-байтовое количество, а 8-байтовые количества выровнены по 8-байтовымграницы, вы тратите 4 байта после каждого из unsigned int значений.

Однако это всего лишь общая критика, а не источник вашей проблемы.


Субстантивное

Вы, похоже, не инициализируете queue, который вы выделяете с помощью malloc().Поскольку malloc() возвращает неинициализированные данные, они могут содержать что угодно.Прежде чем делать что-либо еще, вы должны превратить его в правильно сформированный элемент очереди - с инициализированными указателями и т. Д.

0 голосов
/ 23 февраля 2011

Полагаю, что head является глобальной переменной.Не знаю, почему ваша функция не находит там хорошего значения, но вы могли бы избежать всех своих проблем, просто не указав ей переменную-указатель:

queue head = {0};

Тогда ваш список всегда будет хорошо определен, и выникогда бы даже не пришлось это проверять.Хотя ваша семантика для вашего списка пуста или нет, изменится.

И Джонатан прав, что в вашем критическом разделе очень много кода.В частности, спать внутри такого раздела - действительно плохая идея.

...