System crash

Hi,

I have a function call "process". I want to call this function in a multi threaded environment. When I call to this function inside a thread it was executed successfully. But the problem is the memory usage of the application was increasing at the same time.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59

#define EXTRA_INFOF_CODES_FINAL_RESPONSE_TRUE            255

struct REQUEST_MESSAGE{
	char messageStarter[4];
	char messageTypeIndicator[4];
	char extraInfo[4];
	int messageCounter;
	char commandCode[4];
	char tocken[18];
	int dataLength;
	char *data;
};

char* process(struct REQUEST_MESSAGE* message,char *respData,int tokenID,int finalResponse){
	char response[1024];
	char tempStr[1024];
	int len = 0;

	strcpy(response,message->messageStarter);
	strcat(response,message->messageTypeIndicator);

	 message->messageCounter++;

	 if(99 <= message->messageCounter){
		message->messageCounter = 1;
	}

	sprintf(tempStr,"%02d",message->messageCounter);
	strcat(response,tempStr);

	sprintf(tempStr,"%02X",finalResponse);
	strcat(response,tempStr);

	strcat(response,message->commandCode);

	sprintf(tempStr,"%016d",tokenID);
	strcat(response,tempStr);

	len = strlen(respData);
	sprintf(tempStr,"%04d",len);
	strcat(response,tempStr);

	strcat(response,respData);

	return strdup(response);
}


void* threadFunction(void *threadId){
    // this is the thread (thread pool)
        char data[16];
        struct REQUEST *request_obj = getNextObjectFromQueue();
        strcpy(data,"SUCCESS");
        char *response = process(request_obj->reqMessage,data,0,EXTRA_INFOF_CODES_FINAL_RESPONSE_TRUE);
        free(response);
        response = 0;
}


Any idea??

Thank you.
But the problem is the memory usage of the application was increasing at the same time.
Increased by how much?

You could improve the safety by replacing strcpy/strcat/sprintf with strncpy/strncat/snprintf.

You can also increase the efficiency by passing in response and its size as a parameter, avoiding a call to strdup/free, and make a single call to snprintf rather than 4.

Also, you need a lock around
1
2
3
4
	message->messageCounter++;
	if (99 <= message->messageCounter) {
		message->messageCounter = 1;
	}

which probably should be:
1
2
3
	if (++message->messageCounter > 99) {
		message->messageCounter = 1;
	}
Last edited on
getNextObjectFromQueue() returns a pointer to a struct. Where is that memory freed ? You leak that memory every time threadFunction runs.

You can verify that is your process() function as you have tought by just commenting out lines 55 - 57 in the above code to see what happens.
Last edited on
Topic archived. No new replies allowed.