I have noticed the program I have been testing leaks memory at two different points, no matter if I call free()
or not. I don't understand why it's happening and how it could be fixed.
I am using leaks to test for memory leaks: no Valgrind available for my machine.
Here's the report:
leaks Report Version: 4.0, multi-line stacks
Process 21334: 226 nodes malloced for 18 KB
Process 21334: 2 leaks for 544 total leaked bytes.
STACK OF 1 INSTANCE OF 'ROOT LEAK: <malloc in tokenizer>':
3 libsystem_pthread.dylib 0x18d51eda0 thread_start + 8
2 libsystem_pthread.dylib 0x18d523fa8 _pthread_start + 148
1 a.out 0x102c13c10 tokenizer + 36
0 libsystem_malloc.dylib 0x18d364d88 _malloc_zone_malloc_instrumented_or_legacy + 128
====
1 (272 bytes) ROOT LEAK: <malloc in tokenizer 0x120004080> [272]
STACK OF 1 INSTANCE OF 'ROOT LEAK: <calloc in printwords>':
3 libsystem_pthread.dylib 0x18d51eda0 thread_start + 8
2 libsystem_pthread.dylib 0x18d523fa8 _pthread_start + 148
1 a.out 0x102c13d4c printwords + 40
0 libsystem_malloc.dylib 0x18d364eb0 _malloc_zone_calloc_instrumented_or_legacy + 92
====
1 (272 bytes) ROOT LEAK: <calloc in printwords 0x11ef041d0> [272]
It is a multi-threaded program that reads a text file, separates the words and then prints them. This version does not print the words but the memory leaks are the exact same (I've tested it).
Code:
#include<stdio.h>
#include<stdlib.h>
#include<unistd.h>
#include <pthread.h>
#include <string.h>
#include "functs.h"
pthread_t *thpool;
Deque_t* deque_12;
Deque_t* deque_23;
void* readfile(void* arg);
void* tokenizer();
void* printwords();
char rd;
char tk;
void init(){
deque_12 = new_deque();
deque_23 = new_deque();
rd = 0; //lines read
tk = 0; //all lines tokenized
}
int main(int argc, char *argv[]){
char* filename = "loremIpsum.txt";
init();
thpool = malloc(sizeof(pthread_t)*3);
thread_create(&thpool[0],/*attrs*/ NULL, readfile, (void*)filename);
thread_create(&thpool[1],/*attrs*/ NULL, tokenizer, /*args*/NULL);
thread_create(&thpool[2],/*attrs*/ NULL, printwords, /*args*/NULL);
for(int i=0; i<3; i++)
thread_join(thpool[i], NULL);
free_deque(deque_12,1);
free_deque(deque_23,0);
free(thpool);
}
void* readfile(void* arg){
FILE* fp= fopen((char*)arg, "r");
char* line= calloc(1,sizeof(char)*BUFSIZE);
while(fgets(line, BUFSIZE, fp)!=NULL){
push_tail(deque_12, (void*)line);
line= calloc(1,sizeof(char)*BUFSIZE);
}
rd=1;
printf("1: File read\n");
fclose(fp);
free(line);
return NULL;
}
void* tokenizer(){
struct timespec t = {0,1000000};
while(isEmpty(deque_12))
nanosleep(&t, NULL);
char* bfr = malloc(sizeof(char)*BUFSIZE); //memory leak
while(1){
pop_head(deque_12, (void*)&bfr,1);
if(bfr==NULL && rd) break;
else if(bfr==NULL){
nanosleep(&t, NULL);
continue;
}
char* state;
char* token = strtok_r(bfr, " ",&state);
while(token!=NULL){
push_tail(deque_23, (void*)token);
token = strtok_r(NULL, " ",&state);
}
}
free(bfr);
tk=1;
puts("2: Tokenized");
return NULL;
}
void* printwords(){
struct timespec t = {0,1000000};
char* word= calloc(1, sizeof(char)*BUFSIZE); //memory leak
while(1){
pop_head(deque_23, (void*)&word,0);
if(word==NULL && tk) break;
else if(word==NULL){
nanosleep(&t, NULL);
continue;
}
printf(">%s\n",word);
fflush(stdout);
}
puts("3: all words printed");
free(word);
return NULL;
}
functs.h:
#ifndef _FUNCTS_H
#define _FUNCTS_H
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include<unistd.h>
#include <errno.h>
#ifndef BUFSIZE
#define BUFSIZE 256
#endif
void thread_create(pthread_t *tid,const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg){
int err;
if((err = pthread_create(tid, attr, start_routine, arg)) != 0){
perror("pthread_create");
errno = err;
pthread_exit(&errno);
}
}
void thread_join(pthread_t tid, void **retval){
int err;
if((err = pthread_join(tid, retval)) != 0){
fprintf(stderr,"FATAL_ERROR: thread_join");
errno = err;
pthread_exit(&errno);
}
}
void mtx_lock(pthread_mutex_t *mtx_ptr){
int err;
if((err = pthread_mutex_lock(mtx_ptr)) != 0){
puts("mutex_lock");
errno = err;
pthread_exit(&errno);
}
}
void mtx_unlock(pthread_mutex_t *mtx_ptr){
int err;
if((err = pthread_mutex_unlock(mtx_ptr)) != 0){
perror("mutex_unlock");
errno = err;
pthread_exit(&errno);
}
}
typedef struct node{
void* data;
struct node *next;
struct node *prev;
}Node_t;
typedef struct deque{
Node_t *head;
Node_t *tail;
pthread_mutex_t mtx;
}Deque_t;
Deque_t* new_deque(){
Deque_t* d = malloc(sizeof(Deque_t));
d->head = NULL;
d->tail = NULL;
pthread_mutex_init(&(d->mtx), NULL);
return d;
}
int isEmpty(Deque_t *d){
return (d->head == NULL);
}
void push_tail(Deque_t* d, void* data){
Node_t *newNode = malloc(sizeof(Node_t));
newNode->data = data;
newNode->next = NULL;
mtx_lock(&(d->mtx));
if(isEmpty(d)){
newNode->prev = NULL;
d->head = newNode;
}
else{
newNode->prev = d->tail;
d->tail->next = newNode;
}
d->tail = newNode;
mtx_unlock(&(d->mtx));
}
void pop_head(Deque_t* d, void** val, int ismallocd){
mtx_lock(&(d->mtx));
if(isEmpty(d)){
mtx_unlock(&(d->mtx));
*val = NULL;
return;
}
Node_t *tmp = d->head;
*val = d->head->data;
if(d->head == d->tail){
d->head = NULL;
d->tail = NULL;
}
else{
d->head = d->head->next;
d->head->prev = NULL;
}
mtx_unlock(&(d->mtx));
if(ismallocd) free(tmp->data);
free(tmp);
}
void free_deque(Deque_t* d, int ismallocd){
Node_t* tmp;
while(d->head != NULL){
tmp = d->head;
d->head = d->head->next;
if(ismallocd) free(tmp->data);
free(tmp);
}
free(d);
}
#endif
In your printwords
function, you allocate memory in the following line:
char* word= calloc(1, sizeof(char)*BUFSIZE);
You attempt to free that memory near the end of the function, with:
free(word);
However, by that point (i.e. when the while(1)
loop has been broken), the word
pointer must be NULL
, because the only way out of that loop is in the if(word==NULL && tk) break;
line. Remember that calling the free
function with a NULL
argument is not an error and has well-defined (by the C Standard) behaviour: It does nothing.
I'm not entirely sure why you make that allocation (possibly, so that you can pass a non-null pointer to the pop_head
function); however, if you want to do that, you should allocate it to a different variable and then copy that to the word
pointer used in the loop.
Here's a way you can do this:
void* printwords()
{
struct timespec t = {0,1000000};
char* temp = calloc(1, sizeof(char)*BUFSIZE);
char* word = temp; // We can use that local allocation but we MUST remember it!
while(1) {
pop_head(deque_23, (void*)&word,0);
if(word==NULL && tk) break;
else if(word==NULL){
nanosleep(&t, NULL);
continue;
}
printf(">%s\n",word);
fflush(stdout);
}
puts("3: all words printed");
// free(word); // WRONG! At this point, "word" is NULL
free(temp); // Should work!
return NULL;
}
You could, alternatively, handle the free()
for that allocated char
buffer inside the pop_head
function (e.g., with free(*val)
before you set *val = NULL;
and return), but that may interfere with the proper working of your list deletion process.
Note that you have the exact same problem with your bfr
buffer, in the tokenizer
function, and the remedy is the same.