1

I'm writing a program in C++ using WINAPI to monitor certain directory for new files arriving, and send them in certain order. The files are derived from a live video stream, so there are 2 files in a unit - audio file and video file, and units should be sent in sequence. a. k. a. (1.mp3, 1.avi); (2.mp3, 2.avi)... Architecture is:

1) detect a new file added to the folder, insert file name to the input queue

2) organize files into units, insert units into unit queue

3) send unit by unit

But since I have to use monitoring file directory for files added there, I need to make sure that file is complete, a. k. a. it is ready to send, since the signal appears when the file is created, but it has yet to be filled with info and closed. So I pop file name from a input queue either when queue has more than 1 file (a. k. a. signal came for next file created, that means that previous file is ready to send) or on timeout(10 sec) so for 10 seconds any file should be done.

So in general this program runs and works properly. But, if I assume that the send procedure will take too long time, so the unit queue will grow. And after some number of units buffered in a unit queue the bug appears.

    time[END] = 0;
    time[START] = clock();
    HANDLE    hIOMutex2= CreateMutex (NULL, FALSE, NULL);
        WaitForSingleObject( hIOMutex2, INFINITE );
        hTimer = CreateThread(NULL, 0, Timer, time, 0, &ThreadId1);
        if(hTimer == NULL)
            printf("Timer Error\n");
    ReleaseMutex(hIOMutex2);
    ReadDirectoryChangesW(hDir, szBuffer, sizeof(szBuffer) / sizeof(TCHAR), FALSE, FILE_NOTIFY_CHANGE_FILE_NAME, &dwBytes, NULL, NULL);
    HANDLE    hIOMutex= CreateMutex (NULL, FALSE, NULL);
        WaitForSingleObject( hIOMutex, INFINITE );
        time[END] = clock();
        TerminateThread(hTimer, 0);
    ReleaseMutex( hIOMutex);

After around 800 units buffered in a queue, my program gives me "Time Error" message, if I'm right that means that program can't allocate thread. But in this code program terminates timer thread exactly after the file was created in a directory. So I'm kind of confused with this bug. Also interesting is that even with this time error, my program continue to send units as usual, so that doesn't look like a OS mistake or something different, it is wrong thread declaration/termination, at least it seems like that to me. Also providing Timer code below if it is helpful.

DWORD WINAPI Timer(LPVOID in){
clock_t* time = (clock_t*) in;
while(TRUE){
    if(((clock() - time[START])/CLOCKS_PER_SEC >= 10) && (!time[END]) && (!output.empty())){
        Send();
        if(output.empty()){
            ExitThread(0);
        }
    }
    else if((output.empty()) || (time[END])){
        break;
    }
    else{
        Sleep(10);
    }
}
ExitThread(0);
return 0;
}

Please could anyone here give me some advise how to solve this bug? Thanks in advance.

  • Calling `TerminateThread()` is a very bad idea. I don't know whether it causes your problem, but you shouldn't call it on regular basis. – sharptooth Aug 05 '11 at 08:15
  • hm... is there any other command how I can terminate child? I've seen that terminateThread() is too rough to call too often, but I haven't found any other command for it. – Constantine Samoilenko Aug 05 '11 at 08:17
  • 1
    You can't forcibly terminate a thread and be safe. Either you expect the thread to end itself or you run a separate process and terminate the latter. Expecting the thread to end is not that bad - you use a shared variable with interlocked access and the thread periodically checks that variable and once it is set from the main thread the other thread exits. – sharptooth Aug 05 '11 at 08:20
  • 800 threads in any program is not a good idea. Can't you create a set of worker threads which work on the input queue? They will have to compete with each other on the input queue---so the lesser they are, the better. – Jaywalker Aug 05 '11 at 08:21
  • Don't use `CreateThread()` unless you know exactly what you're doing. It's probably irrelevant to your current situation, but it may create issues for you later on. See this discussion: http://stackoverflow.com/questions/331536/windows-threading-beginthread-vs-beginthreadex-vs-createthread-c – Jörgen Sigvardsson Aug 05 '11 at 08:38
  • 2 sharptooth: hm... strange that I haven't done this before, seems like time[END] was exactly for this purpose :) Thank You for hint. 2 Jaywalker: I'm sorry, I wasn't clear enough, English is not my native language... I meant that after 800 units are buffered in an unit queue, not 800 threads. Concerning about threads - I tried to make program with least resource consumption, so that's one of the reasons I tried to terminate child, to release resources asap. – Constantine Samoilenko Aug 05 '11 at 08:38
  • Ignoring the whole thread issue, can you determine if the file has finished by seeing if it's locked/larger then a given size? This would save the whole "wait for the next one" issue. – Deanna Aug 05 '11 at 09:25

1 Answers1

4

Using TerminateThread is a bad idea in many ways. In your case, it makes your program fail because it doesn't release the memory for the thread stack. Failure comes when your program has consumed all available virtual memory and CreateThread() cannot reserve enough memory for another thread. Only ever use TerminateThread while exiting a program.

You'll have to do this a smarter way. Either by asking a thread to exit nicely by signaling an event or by just not consuming such an expensive system resource only for handling a file. A simple timer and one thread can do this too.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536