C - Частота подсчета букв в многопоточном режиме вызывает ошибку памяти

Я пытаюсь использовать многопоточность C, чтобы узнать частоту каждой буквы алфавита в текстовом файле. Задание состоит в том, чтобы: 1) написать функцию, которая читает каждое предложение в тексте, оканчивающееся символом '.' 2) напишите функцию, которая загружает предложение в двумерный массив 3) напишите функцию, которая генерирует pthread для каждой буквы для каждого предложения (функция pthread добавляет 1 к счетчику для этой буквы). РЕДАКТИРОВАТЬ: с помощью Valgrind я понял, что проблема в функции sentence, но я не понимаю, почему.

Вот код:

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <pthread.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/types.h>

char alphabet[26] = "abcdefghijklmnopqrstuvwxyz";
int count[26];

char* sentence(char * s){
    char* p;
    char* q;
    char* arr;
    int i;
    p = s;
    q = malloc(100);
    arr = q;
    for (i=0; *p != '.'; i++){ 
        *q = *p;
        q++;
        p++; 
    }
    *q = '\0';
    return arr;
}

char** load_sentence(char* p, char** q, int i){
    q[i] = malloc(strlen(p)+1);
    strcpy(q[i], p);
    return q;
}

void* count_letter(void * s){
    char* p = (char*) s;
    int i;
    for (i=0; i<26; i++){
        if (*p == alphabet[i]){
            count[i]++;
        }
    }
}

void frequency(char* str){
    char* s = str;
    int i, j, l;
    l = strlen(str);
    pthread_t tid[l];
    for (i=0; i<l; i++){
        pthread_create(&tid[i], NULL, count_letter, (void*) s);
        s++;
    }
    for (j=0; j<l; j++){
        pthread_join(tid[j], NULL);
    }
}


int main(int argc, char* argv[]){

    int fd;
    char buff[100];
    fd = open(argv[1], O_RDONLY);
    char ** text = malloc(10*sizeof(char*));
    read(fd, buff, sizeof(buff));
    char* start = buff;
    int i = 0; //number of phrases!
    char* p = NULL;

    while (*(p = sentence(start)) != '\0'){
        text = load_sentence(p, text, i);
        start += strlen(p)+1;
        i++;
   }

   int j, k;

   for (k=0; k<i; k++){
        frequency(text[k]);
   }

   for (j=0; j<26; j++){
        printf("%c : %d times\n", alphabet[j], count[j]);
   }
}

Похоже, что с такими случаями: hope it's a good reading. bye. Вывод правильный:

a : 2 times
b : 1 times
c : 0 times
d : 2 times
e : 3 times
f : 0 times
g : 3 times
h : 1 times
i : 2 times
j : 0 times
k : 0 times
l : 0 times
m : 0 times
n : 1 times
o : 3 times 
p : 1 times
q : 0 times
r : 1 times
s : 1 times
t : 1 times
u : 0 times
v : 0 times
w : 0 times
x : 0 times
y : 1 times
z : 0 times

У других — «ошибка памяти», начинающаяся с free() : invalid next size (normal). Ошибка имеет много строк карты памяти и заканчивается абортом.

Я совсем новичок в C, извините за мою неопытность.

Нужно ли вводить mutex в этом случае?


person erika    schedule 17.10.2018    source источник
comment
C позволяет использовать более одного символа для имен переменных. Используй их. И если вы не вынуждены использовать C89, но C99 или выше, объявите и определите переменные, где они используются. Затем ознакомьтесь с вашим любимым отладчиком. Ваш load_sentence() не делает того, о чем говорит его название.   -  person Swordfish    schedule 17.10.2018
comment
У меня он работает в ideone: ideone.com/QVOALf с вашим вводом aaaaaaaaa.aaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaa.   -  person izlin    schedule 17.10.2018
comment
вы портите память. ваш text имеет место только для 10 указателей. хотя ваш load_sentence может получить доступ ко многим, равным длине предложения, более 10 для aaaa...   -  person Serge    schedule 17.10.2018
comment
Я знаю, что C позволяет использовать более одного символа для имен переменных, мне жаль, что это не так читабельно. Но это не проблема, верно? Почему вы говорите, что функция load_sentence() не делает того, что следует из ее названия? Он действительно копирует предложение, указанное указателем p, в двумерный массив q в позиции i.   -  person erika    schedule 17.10.2018
comment
Серж, можешь лучше объяснить, что ты хочешь сказать? Я знаю, что указателей всего 10, но они, указатели на указатели, значит, таким образом этот код может прочитать только 10 предложений...   -  person erika    schedule 17.10.2018
comment
Вам нужно обнулить их там строки. char alphabet[26+1] = "abc...   -  person Lundin    schedule 17.10.2018
comment
Например, вы столкнулись с этой маленькой ловушкой: stackoverflow.com/a/52385480/584518   -  person Lundin    schedule 17.10.2018
comment
Спасибо за ваш комментарий, Лундин, но кажется, что модификация не решает проблему...   -  person erika    schedule 17.10.2018
comment
Я предполагаю, что это школьный проект по изучению многопоточного программирования? если нет ... удалите мьютекс, и его потоковая обработка не ускорит процесс, поскольку он написан кодом ... Также почему вы читаете предложение за предложением, когда все равно представляете данные плоскими?   -  person Anders Cedronius    schedule 17.10.2018
comment
Да, школьная заявка... К сожалению, наш профессор не очень помог. Просьба читать текст таким образом, предложение за предложением.   -  person erika    schedule 17.10.2018


Ответы (2)


Ваша предыдущая версия с mutex имела неопределенное поведение, поскольку вы несколько раз инициализировали мьютекс, согласно ссылке. :

Попытка инициализировать уже инициализированный мьютекс приводит к неопределенному поведению.

Вы получаете доступ к count одновременно, поэтому вам нужно использовать мьютекс для создания потокобезопасного кода. Вы назвали pthread_mutex_init в count_letter, это неверно, эта функция является телом вашего потока (многократная инициализация мьютекса без его разрушения приводит к UB), вы должны вызывать pthread_mutex_init только один раз, например, как первую строку в основной функции:

int main() {
 pthread_mutex_init(&mtx,NULL);

перед возвращением добавить

 pthread_mutex_destroy(&mtx);

Критический раздел в вашей функции count_letter — строка

count[i]++;

вы должны изменить его следующим образом

pthread_mutex_lock(&mtx);
count[i]++;
pthread_mutex_unlock(&mtx);

Теперь вернемся к реализации sentence, вам нужно проверить, не указывает ли *p на нулевой терминатор перед сравнением с .:

for (i=0; *p && *p != '.'; i++){ 
          ^^ added

без проверки \0 != . возвращает true, и ваш цикл продолжается...

person rafix07    schedule 17.10.2018
comment
Спасибо rafix07 за объяснение! Мне действительно было трудно понять. Управление памятью с помощью C не так просто...! Я добавил этот код, и теперь он работает нормально. Спасибо :) - person erika; 18.10.2018

Эрика,

Поскольку я действительно не знаю вашего задания, пожалуйста, рассматривайте это как еще один способ подсчитать 1000 символов. На баги не проверял, перепишите под свои нужды. Во всяком случае, я бы так решил. Если памяти мало, я бы читал символ за символом из файла до «.». В любом случае надеюсь, что это поможет вам, и вы получите отличные оценки :-)...

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <pthread.h>
#include <stdatomic.h>

#define MAX_THREADS 100
atomic_int threadCount;
#define NCHAR 26
char alphabet[NCHAR] = "abcdefghijklmnopqrstuvwxyz";
atomic_int count[NCHAR];


void* count_letter(void * s){
    threadCount++;
    char* p = (char*) s;
        for (int i=0; i<NCHAR; i++)
            if (*p == alphabet[i])
                count[i]++;
    threadCount--;
    return NULL;
}

int main(int argc, char* argv[]){

    //Init variables
    FILE *file;
    char *myText;
    unsigned long fileLen;
    int deadLockGuard=0;
    threadCount=0;

    //Open the file
    file = fopen(argv[1], "rb");
    if (!file) {
        fprintf(stderr, "Unable to open file %s", argv[1]);
        return EXIT_FAILURE;
    }
    fseek(file, 0, SEEK_END);
    fileLen=ftell(file);
    rewind(file);

    //reserve memory and read the file
    myText=(char *)malloc(fileLen+1);
    if (!myText) {
        fprintf(stderr, "Memory error!");
        fclose(file);
        return EXIT_FAILURE;
    }
    fread(myText, fileLen, 1, file);
    fclose(file);

    //Get each sentence ending with a . and then for each character look at the count for each character in it's own thread.
    char *subString = strtok(myText, "."); //This is your sentence/load_sentence method
    while (subString != NULL) {
        for (int v = 0;v<strlen(subString);v++) { //This is your frequency method
        deadLockGuard=0;
        while (threadCount >= MAX_THREADS) {
            usleep(100); //Sleep 0.1ms
            if(deadLockGuard++ == 10000) {
                printf("Dead-lock guard1 triggered.. Call Bill Gates for help!"); //No free threads after a second.. Either the computer is DEAD SLOW or we got some creepy crawler in da house.
                return EXIT_FAILURE;
            }
        }

        pthread_t tid; //Yes you can overwrite it.. I use a counter to join the workers.
        pthread_create(&tid, NULL, count_letter, (void*) subString+v);
    }
        subString = strtok(NULL, ".");
    }
    deadLockGuard=0;
    //pthread_join all the still woring threads
    while (threadCount) {
        usleep(1000); //sleep a milli
        if(deadLockGuard++ == 2*1000) {
            printf("Dead-lock guard2 triggered.. Call Bill Gates for help!"); //Threads are running after 2 seconds.. Exit!!
            return EXIT_FAILURE;
        }
    }
    //Garbage collect and print the results.
    free(myText);
    for (int j=0; j<NCHAR; j++)
        printf("%c : %d times\n", alphabet[j], count[j]);
    return EXIT_SUCCESS;
}
person Anders Cedronius    schedule 17.10.2018
comment
Anders Cedronius, спасибо за ответ на эту проблему, это было полезно. К сожалению, задание требует этих проходов, поэтому я ввожу эти функции ;( В любом случае, спасибо! :) - person erika; 17.10.2018