Win API WaitForMultiple Thread Crashing Application

[Keozon]

So, I've been having no end to random inconsistencies that did not make any sense to me using QFileSystemWatcher (see my second-to-last post "here":http://qt-project.org/forums/viewthread/17216/ for details), so I decided I was going to go to the source and just use Win API. Which, at first, worked. Then, at some point it started crashing. I know I must have made some small modifications before it did so, but I really don't remember changing anything. I even used version control software to find the differences in the code for me, and I couldn't see anything that could possibly cause the problem. By using excessive debug outputs, I've determined that the thread always crashes right when it enters the WaitForMultipleObjects event loop.

I have reversed all my changes in that loop to their original state, when it first worked, and it still crashes. I've cleaned all. I've restarted my computer. Nothing seems to fix it. I've spent several hours tweaking, compiling and rechecking to no avail. I've decided I need another set of eyes.

Here is the applicable code:
@
sharedData a;
/The File Monitoring thread. This thread waits for a directory change to do anything./
FileMon::FileMon(){
}
QStringList FileMon::getFileList(QString arg){
QDir dir(arg);
QStringList files = dir.entryList(QDir::Files,QDir::Time);
return files;
}
void FileMon::run(){
emit newDebug("Thread started successfully. Working dir: "+a.getDir());
monitor.addPath(a.getDir());
QStringList files = getFileList(a.getDir());
if (!files.isEmpty()){
const QString fullyQ = a.getDir();
emit newDebug("Watching most recent file: " + files[0]);
watchFile(fullyQ);
}
else {
emit newDebug("Directory is empty. Will add new file watcher when file is added.");
}
QObject::connect(&monitor,SIGNAL(directoryChanged(QString)),SLOT(change_notify(QString)));
QObject::connect(&monitor,SIGNAL(fileChanged(QString)),SLOT(change_notify(QString)));
exec();
}
bool FileMon::setconnect(QPushButton *arg,TorTools *arg2){
bool result = QObject::connect(arg,SIGNAL(clicked()),this,SLOT(terminate()));
if (result){result = QObject::connect(this,SIGNAL(newDebug(QString)),arg2,SLOT(onNewDebug(QString)));}
if (result){result = QObject::connect(this,SIGNAL(sendNotify(QString)),arg2,SLOT(on_directory_change(QString)));}
return result;
}
void FileMon::change_notify(QString arg){
emit sendNotify(arg);
/This checks to see if a new file was added, or if the currently watched file was changed.
if a new file was added, it removes the old one (if any) and replaces it with the new one./
if (arg == a.getDir()){
QStringList newFiles = getFileList(a.getDir());
emit newDebug("Now watching new file "+newFiles[0]);
const QString fullyQ = a.getDir();
watchFile(fullyQ);
}
}
void FileMon::watchFile(const QString arg){
WCHAR *fname;
arg.toWCharArray(fname);
HANDLE changeNotifications[2];
DWORD waitStatus;
bool fileAdded = false;
//detect changes to watched file size, signifying write
changeNotifications[0] = FindFirstChangeNotification(fname,false,FILE_NOTIFY_CHANGE_SIZE);
//detect changes to the directory (file added) to signify new file to watch
changeNotifications[1] = FindFirstChangeNotification(fname,false,FILE_NOTIFY_CHANGE_FILE_NAME);
if (changeNotifications[0] == INVALID_HANDLE_VALUE
|| changeNotifications[1] == INVALID_HANDLE_VALUE){
emit newDebug("Error: Invalid file handle.");
}
else if (changeNotifications[0] == NULL
|| changeNotifications[1] == NULL){
emit newDebug("Error: Unexpected NULL handle.");
}
//start waiting in infinite loop. Thread is terminated when logging turned off, will kill loop forcibly.
//bad form? Perhaps. But, this thread makes no actual changes to files or data inside loop.
else {
emit newDebug("Now starting API FS watch.");
while (!fileAdded){
waitStatus = WaitForMultipleObjects(2,changeNotifications,false,INFINITE);
switch (waitStatus)
{
case WAIT_OBJECT_0:
//watched file was written to
emit newDebug("Watched File Changed!");
if (!FindNextChangeNotification(changeNotifications[0])){
emit newDebug("Unexpected error when attempting to continue file monitoring.");
}
break;
case WAIT_OBJECT_0 + 1:
//File added to directory
fileAdded = true;
break;
}
}
}
}
@

And here is the output before it crashes:
@
"Starting Thread..."
"Connections Made."
"Thread started successfully. Working dir: C:\Users\OMITTED"
"Watching most recent fileNew Text Document.txt"
The program has unexpectedly finished.
C:\Users\OMITTED\TorTools.exe exited with code -1073741819
@
The error code is not consistent.

[Keozon]

Okay, I figured it out. It was (surprise, surprise) a memory management issue. line 44 needed to change to:
@
WCHAR *fname = new WCHAR[arg.length];
@

Oddly, I'm 100% sure that worked before without the allocation. Oh well, I'm content that it's fixed.

[Wilk]

Hello.
Actualy it's totaly your fault. For more information read documentation for "toWCharArray":http://qt-project.org/doc/qt-4.8/qstring.html#toWCharArray method.

[MuldeR]

[quote author="Keozon" date="1337204619"]Okay, I figured it out. It was (surprise, surprise) a memory management issue. line 44 needed to change to:
@
WCHAR *fname = new WCHAR[arg.length];
@

Oddly, I'm 100% sure that worked before without the allocation. Oh well, I'm content that it's fixed.[/quote]

Actually I think you should modify the code like this:
@const size_t len = arg.length();
WCHAR *fname = new WCHAR[len + 1];
arg.toWCharArray(fname);
fname[len] = L'\0';@

That's because, according to the docs, toWCharArray() does NOT append the required NULL-Terminator!
Nor does arg.length() include the extra space for the trailing NULL character.

You could also do this, which avoids the requirement to make a temp copy of the string:
@FindFirstChangeNotification((const wchar_t*) arg.utf16(), false, FILE_NOTIFY_CHANGE_SIZE);@

(The typecast is needed because Qt uses "ushort*" to store UTF-16 chars, while the Win32 API uses "wchar_t*". But don't worry, they're both "16-Bit per character" types with UTF-16 encoding, at least on Win32)

[Keozon]

Thanks for the typecast form! That helped simplify the code quite a bit.
Also, I had read about the NULL terminator issue, but it didn't seem to cause any issues, so I ignored it.

[MuldeR]

[quote author="Keozon" date="1337268649"]Also, I had read about the NULL terminator issue, but it didn't seem to cause any issues, so I ignored it.[/quote]

If your old code, which did not care about the required NULL terminator, worked, then this was sheer luck...