-
January 10th, 2014, 07:20 PM
#1
Memory leak in multithreaded program
Hello,
A while ago I wrote a program to interface with a scientific instrument. The program works as intended but if I leave it on for long periods of time it behaves sluggishly, and task manager shows it using hundreds of MB of memory (it should use very little). I am wondering if someone could look at a stripped down version of my code showing the bare essentials and suggest what I could be doing incorrectly?
The basic strategy of the program is as follows:
- to send a command to the instrument, I have to send it a windows message
- to obtain a response from the instrument, I have to listen for a windows message
- I need to simultaneously listen for status updates while executing a function (the function depends on those status updates to execute correctly); therefore, I need to multithread
- I wrap the variables that I need in the new thread in a mutex to avoid access violations
The code looks as follows:
Code:
#includes here
// status updates
#define IDT_CHECKSTATUS 333
#define IDT_REFRESHSCREEN 334
bool receiving_data = false; // Flag indicates that next message from instrument will contain data rather than status update
HANDLE StatusMutex; // multithreading
// MAIN FUNCTION
int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow)
{
// window initialization, etc is here
// set timer for CheckStatus (function that asks instrument to report its status)
TimerID = SetTimer(main_hwnd,IDT_CHECKSTATUS, (UINT) 0.01* 1000, NULL);
// set timer to refresh screen
SetTimer(main_hwnd, IDT_REFRESHSCREEN, 1 * 1000, NULL);
while(GetMessage(&Msg, NULL, 0, 0) > 0)
{
TranslateMessage(&Msg);
DispatchMessage(&Msg);
}
return Msg.wParam;
}
// EVENT HANDLER
LRESULT CALLBACK WndProc(HWND main_hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
switch(msg)
{
// Omitting some standard messages such as WMCreate
case WM_PAINT:
// output the status to the window
{
RECT rcClient;
PAINTSTRUCT ps;
HDC hdc = BeginPaint(main_hwnd, &ps);
GetClientRect(main_hwnd, &rcClient);
DrawStatusUpdate(hdc, &rcClient, g_hfFont);
EndPaint(main_hwnd, &ps);
}
break;
case WM_TIMER: // Timer has ticked!
{
switch (wParam)
{
case IDT_CHECKSTATUS:
{
CheckStatus(main_hwnd); // asks instrument for its status using current thread (no new thread created)
// Some other maintenance work here
break;
}
case IDT_REFRESHSCREEN:
InvalidateRect(main_hwnd, NULL, FALSE); // cause a paint message
break;
}
}
break;
case WM_COPYDATA: // CopyDataStruct incoming from the instrument!
{
// Some pre-processing to obtain string str from the WM_COPYDATA lpdata
ParseStatus(str, main_hwnd); // interprets message as status update or data
}
break;
case WM_COMMAND: // One of these (the one below) sends a command to the instrument IN A NEW THREAD
switch(LOWORD(wParam))
{
case ID_FILE_EXECUTEPROCEDURE:
{
exec_Parms *To_Pass = new exec_Parms();
To_Pass->hwnd = main_hwnd;
To_Pass->parsed_Commands = parsed_Commands;
CreateThread(NULL, 0,SendRequestToInstrument, To_Pass, 0, NULL); // SendRequestToInstrument is the function being executed in a new thread
}
break;
}
}
return 0;
}
void ParseStatus(const char* status, HWND main_hwnd)
{
// Some string formatting here
// create a vector<std:string> called "info", populate it with components of the status string
if ( (PSEUDOCODE) status string actually contains a status, rather than data)
{
// mutex
StatusMutex = CreateMutex(
NULL, // default security attributes
FALSE, // initially not owned
NULL); // unnamed mutex
if (StatusMutex == NULL)
{
printf("CreateMutex error: %d\n", GetLastError());
return;
}
DWORD dwWaitResult = WaitForSingleObject(
StatusMutex, // handle to mutex
INFINITE); // no time-out interval
switch (dwWaitResult)
{
// The thread got ownership of the mutex
case WAIT_OBJECT_0:
// Code here extracts information from the info vector & assigns it to global variables used elsewhere in the program
// Release ownership of the mutex object
if (! ReleaseMutex(StatusMutex))
{
// Handle error.
}
break;
// The thread got ownership of an abandoned mutex
// The database is in an indeterminate state
case WAIT_ABANDONED:
return;
}
}
else // status string contains DATA rather than a status update
{
ParseData(info) // how this works not important
receving_data = false; // flag: data reception is complete.
}
return;
}
void SendRequestToInstrument(string command)
{
receiving_data = true; // Flag to expect incoming data and wait to receive it before continuing
// code to send data to the instrument goes here
while (receiving_data == true) { Sleep(100); } // Pause execution of this thread until event handler has acknowledged receipt of data
// do things with the data
}
Is there anything apparently incorrect with this code?
Last edited by Danja91; January 10th, 2014 at 07:28 PM.
-
January 10th, 2014, 08:01 PM
#2
Re: Memory leak in multithreaded program
Each call to CreateMutex() should have a corresponding CloseHandle() call.
I also don't see the "delete" of "exec_Parms".
gg
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|