0

Hi I am using C++ with the Qt framework for a Windows program. I use Qt threads but this situation could relevant for other thread APIs as well. I am dedicating a worker thread to monitor for directory changes using ReadDirectoryChangesW and WaitForMultipleObjects from the Win API. I want to be able to cancel the worker thread gracefully from the main thread. I have read about CancellIOEx that takes a handle and OVERLAPPED parameter but these data types are both pointers. Is there some safe way to pass these pointers from the worker thread to the main thread safely? Is there a better way of doing things?

Here's some code from here but using WaitForSingleObject instead of WaitForMultipleObjects, the function will be called from the worker thread. Am I allowed to post this code from the link? Also see here for valuable information about ReadDirectoryChangesW outside of the Windows Dev Center.

Thanks!

void WatchDirectory(LPCWSTR path)
{
   char buf[2048];
   DWORD nRet;
   BOOL result=TRUE;
   char filename[MAX_PATH];
   DirInfo[0].hDir = CreateFile (path, GENERIC_READ|FILE_LIST_DIRECTORY, 
                                 FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
                                 NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED,
                                 NULL);

   if(DirInfo[0].hDir == INVALID_HANDLE_VALUE)
   {
       return; //cannot open folder
   }

   lstrcpy( DirInfo[0].lpszDirName, path);
   OVERLAPPED PollingOverlap;

   FILE_NOTIFY_INFORMATION* pNotify;
   int offset;
   PollingOverlap.OffsetHigh = 0;
   PollingOverlap.hEvent = CreateEvent(NULL,TRUE,FALSE,NULL);
   while(result)
   {
       result = ReadDirectoryChangesW(
                  DirInfo[0].hDir,// handle to the directory to be watched
                  &buf,// pointer to the buffer to receive the read results
                  sizeof(buf),// length of lpBuffer
                  TRUE,// flag for monitoring directory or directory tree
                  FILE_NOTIFY_CHANGE_FILE_NAME |
                  FILE_NOTIFY_CHANGE_DIR_NAME |
                  FILE_NOTIFY_CHANGE_SIZE,
                //FILE_NOTIFY_CHANGE_LAST_WRITE |
                //FILE_NOTIFY_CHANGE_LAST_ACCESS |
                //FILE_NOTIFY_CHANGE_CREATION,
                &nRet,// number of bytes returned
                &PollingOverlap,// pointer to structure needed for overlapped I/O
                NULL);

       WaitForSingleObject(PollingOverlap.hEvent,INFINITE);
       offset = 0;
       int rename = 0;
       char oldName[260];
       char newName[260];
       do
       {
           pNotify = (FILE_NOTIFY_INFORMATION*)((char*)buf + offset);
           strcpy(filename, "");
           int filenamelen = WideCharToMultiByte(CP_ACP, 0, pNotify->FileName, pNotify->FileNameLength/2, filename, sizeof(filename), NULL, NULL);
           filename[pNotify->FileNameLength/2] = '';
           switch(pNotify->Action)
           {
               case FILE_ACTION_ADDED:
                   printf("\nThe file is added to the directory: [%s] \n", filename);
                   break;
               case FILE_ACTION_REMOVED:
                   printf("\nThe file is removed from the directory: [%s] \n", filename);
                   break;
               case FILE_ACTION_MODIFIED:
                   printf("\nThe file is modified. This can be a change in the time stamp or attributes: [%s]\n", filename);
                   break;
               case FILE_ACTION_RENAMED_OLD_NAME:
                   printf("\nThe file was renamed and this is the old name: [%s]\n", filename);
                   break;
               case FILE_ACTION_RENAMED_NEW_NAME:
                   printf("\nThe file was renamed and this is the new name: [%s]\n", filename);
                   break;
               default:
                   printf("\nDefault error.\n");
                   break;
            }

           offset += pNotify->NextEntryOffset;

        }while(pNotify->NextEntryOffset); //(offset != 0);
      }

    CloseHandle( DirInfo[0].hDir );

}
riverofwind
  • 525
  • 4
  • 17
  • 7
    Add an [event object](https://learn.microsoft.com/en-us/windows/win32/sync/event-objects) to the array of handles you are waiting for, and signal that event, when you want your worker thread to terminate. – IInspectable Apr 17 '20 at 21:53
  • Thanks it's very straightforward stuff fortunately :) Is there a book or some source of information on things like this? The Win API can be somewhat elusive at times... – riverofwind Apr 18 '20 at 03:50
  • You only need 2 books to get up to speed with the Windows API: Charles Petzold's [Programming Windows®, Fifth Edition](https://www.amazon.com/dp/157231995X) explains the essentials of application development. It's dated, but almost all of it still applies today. For systems programming, there's Jeffrey Richter's [Advanced Windows](https://www.amazon.com/dp/1572315482). That's the one I have, but there's a revised publication ([Windows via C/C++](https://www.amazon.com/dp/0135953391/)) that accounts for changes in the OS. You probably want to get the latter. – IInspectable Apr 18 '20 at 08:02
  • @IInspectable Having a small problem with Event objects. I createevent from main thread then openevent from worker thread. I get OpenEvent failed with error 5: Access is denied in the worker thread. Shouldn't the default security descriptor be enough? What am I doing wrong? – riverofwind Apr 20 '20 at 20:12
  • Think I got it... did this the key part being EVENT_ALL_ACCESS OpenEvent(EVENT_ALL_ACCESS,FALSE,TEXT("MainEvent")); – riverofwind Apr 20 '20 at 20:16
  • You don't need `OpenEvent`. Just pass the event handle into your thread procedure. If you do decide to use `OpenEvent` in your worker thread, you really only need `SYNCHRONIZE` access to it. – IInspectable Apr 21 '20 at 07:35
  • Isn't a handle a pointer though? Do you have to worry about race conditions using it in both the main gui thread and the worker thread? – riverofwind Apr 21 '20 at 21:31
  • The underlying type of a `HANDLE` is a pointer. But it doesn't represent a memory address, rather than an index into a system-controlled collection. Since you are passing it by value, there's no opportunity for a data race. The system ensures that all accesses to the referenced resource is synchronized. After all, an event object is a synchronization primitive used for thread synchronization. – IInspectable Apr 22 '20 at 06:00

2 Answers2

1

The main thread could create the OVERLAPPED struct and give it to the thread to use, rather than the other way around. However, trying to cancel the I/O from the main thread would be a race condition either way. Since your worker thread has to make a new call to ReadDirectoryChangesEx() after every directory event, it could be between calls to ReadDirectoryChangesEx() when the main thread wants the worker thread to terminate, thus calling CancelIoEx() when there is no I/O in progress would be a no-op.

Instead, create another event object for the main thread and worker thread to share, in addition to the event object that you are creating for the I/O. Have the worker thread wait on both events at the same time with WaitForMultipleObjects(), and then the main thread can signal the shared event when it wants the worker thread to terminate.

WaitForMultipleObjects() will tell the worker thread which event was signaled. If the shared event is signaled, the worker thread can cancel its I/O in progress via CancelIo/Ex() before exiting.

// shared with both threads...
HANDLE hTermEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
// in main thread...
HANDLE hWorkerThread = CreateThread(...);
...
SetEvent(hTermEvent);
WaitForSingleObject(hWorkerThread, INFINITE);
// called by worker thread...
void WatchDirectory(LPCWSTR path)
{
   DWORD buf[512];
   DWORD nRet, dwRet;
   char filename[MAX_PATH];
   DirInfo[0].hDir = CreateFile(path, GENERIC_READ | FILE_LIST_DIRECTORY, 
                                 FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
                                 NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED,
                                 NULL);

   if (DirInfo[0].hDir == INVALID_HANDLE_VALUE)
   {
       return; //cannot open folder
   }

   lstrcpy(DirInfo[0].lpszDirName, path);

   OVERLAPPED PollingOverlap = {};
   PollingOverlap.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
   if (!PollingOverlap.hEvent)
   {
       return; //cannot create I/O event to wait on
   }

   FILE_NOTIFY_INFORMATION* pNotify;
   int offset;

   do
   {
       if (!ReadDirectoryChangesW(
              DirInfo[0].hDir,// handle to the directory to be watched
              &buf,// pointer to the buffer to receive the read results
              sizeof(buf),// length of lpBuffer
              TRUE,// flag for monitoring directory or directory tree
              FILE_NOTIFY_CHANGE_FILE_NAME |
              FILE_NOTIFY_CHANGE_DIR_NAME |
              FILE_NOTIFY_CHANGE_SIZE,
              //FILE_NOTIFY_CHANGE_LAST_WRITE |
              //FILE_NOTIFY_CHANGE_LAST_ACCESS |
              //FILE_NOTIFY_CHANGE_CREATION,
              &nRet,// number of bytes returned
              &PollingOverlap,// pointer to structure needed for overlapped I/O
              NULL))
       {
           break; // can't wait for an event
       }

       HANDLE events[] = {hTermEvent, PollingOverlap.hEvent};

       dwRet = WaitForMultipleObjects(2, events, FALSE, INFINITE);
       if (dwRet != (WAIT_OBJECT_0 + 1))
       {
           CancelIo(DirInfo[0].hDir);
           GetOverlappedResult(DirInfo[0].hDir, &PollingOverlap, &nRet, TRUE);
           break; // terminate requested, or wait failed
       }

       if (!GetOverlappedResult(DirInfo[0].hDir, &PollingOverlap, &nRet, TRUE))
       {
           break; // read failed
       }

       if (nRet == 0)
       {
           continue; // overflow, current event data discarded
       }

       offset = 0;
       int rename = 0;
       char oldName[MAX_PATH];
       char newName[MAX_PATH];
       do
       {
           pNotify = (FILE_NOTIFY_INFORMATION*) (buf + offset);
           int filenamelen = WideCharToMultiByte(CP_ACP, 0, pNotify->FileName, pNotify->FileNameLength/2, filename, sizeof(filename), NULL, NULL);
           switch (pNotify->Action)
           {
               case FILE_ACTION_ADDED:
                   printf("\nThe file is added to the directory: [%.*s] \n", filenamelen, filename);
                   break;
               case FILE_ACTION_REMOVED:
                   printf("\nThe file is removed from the directory: [%.*s] \n", filenamelen, filename);
                   break;
               case FILE_ACTION_MODIFIED:
                   printf("\nThe file is modified. This can be a change in the time stamp or attributes: [%.*s]\n", filenamelen, filename);
                   break;
               case FILE_ACTION_RENAMED_OLD_NAME:
                   printf("\nThe file was renamed and this is the old name: [%.*s]\n", filenamelen, filename);
                   break;
               case FILE_ACTION_RENAMED_NEW_NAME:
                   printf("\nThe file was renamed and this is the new name: [%.*s]\n", filenamelen, filename);
                   break;
               default:
                   printf("\nDefault error.\n");
                   break;
           }

           offset += pNotify->NextEntryOffset;
       }
       while (pNotify->NextEntryOffset);
   }
   while (true);

   CloseHandle(PollingOverlap.hEvent);
   CloseHandle(DirInfo[0].hDir);
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • *ReadDirectoryChangesW* wait for *pointer to the DWORD-aligned buffer*. (more exactly align as `FILE_NOTIFY_INFORMATION` which is DWORD-aligned). the `char buf[2048];` is it located in stack in my test (with CL compiler) always `DWORD` aligned. but formally this is not mandatory/guaranteed. if `buf` will be member of structure - it really can be not 4-byte align (because *char* array not require any align). so for be formal correct need declare it as `ULONG buf[xxx]` for example. or like i inside union { FILE_NOTIFY_INFORMATION fni; char buf[*];} because all union members have the same align – RbMm Apr 18 '20 at 22:01
  • One more question pestering me... can I reuse the same CreateFile handle for two separate calls to ReadDirectoryChangesW, one for file modifications and one for directory modifications? I tried but the program started behaving unpredictably. I'd like to separate file changes from dir changes since ReadDirectoryChangesW doesn't tell you which has happened. Thanks. – riverofwind Apr 21 '20 at 04:13
  • I also reused the buffer and bytes transferred variables between the two separate calls so that could have caused the trouble... – riverofwind Apr 21 '20 at 04:19
  • @riverofwind "*can I reuse the same CreateFile handle for two separate calls to ReadDirectoryChangesW*" - I don't know, I've never tried, but my gut feeling is no, since the docs say it allocates only a single buffer for the `HANDLE` to track events. But, maybe it will be able to differentiate when different event flags are associated with different `OVERLAPPED` structs. I just don't know. "*I also reused the buffer and bytes transferred variables between the two separate calls*" - definitely don't do that. Two separate operations need two separate sets of variables to write to. – Remy Lebeau Apr 21 '20 at 06:45
  • After some experimenting I found if I reused the same CreateFile handle for both dir change and file change calls to ReadDirectoryChangesW, the first call with the specified handle would work but the second call would never trigger after changes to the file/dir structure of the dir to be monitored. So I tried using a unique CreateFile handle for each ReadDirectoryChangesW call and everything is working perfectly now. – riverofwind Apr 21 '20 at 19:27
0

terminate thread never safe. how minimum this lead to resource leaks. and main - you never can know at which point thread was when you terminate it. it can for example be inside heap alloc/free critical section. and terminate it at this point lead to deadlock on next heap operation (because it critical section will never released).

however exist many correct solutions, how stop I/O. possible of course use 2 special event how already described in comments, but in my opinion this is not the best solution

1) we can use CancelIoEx on file handle. of course simply call CancelIoEx not enough - because at this time can be no I/O active in dedicated thread. need yet use special flag (_bQuit) for task canceled, but even this not enough. need check/set this flag inside critical section or rundown-protection with ReadDirectoryChangesW/CancelIoEx

in dedicated thread

AcquireSRWLockExclusive(this);

if (!_bQuit) // (1)
{
    ReadDirectoryChangesW(*); // (4)
}

ReleaseSRWLockExclusive(this);

and for stop

AcquireSRWLockExclusive(this);

_bQuit = true; // (2)
CancelIoEx(*); (3)

ReleaseSRWLockExclusive(this);

without critical section or rundown-protection will be possible execution in next order:

if (!_bQuit) // (1)
_bQuit = true; // (2)
CancelIoEx(*); (3)
ReadDirectoryChangesW(*); // (4)

can be situation when worked thread first check flag _bQuit and it still false. then main thread set flag and call CancelIoEx which will be have no effect, because no I/O on file. and only then worked thread call ReadDirectoryChangesW whicj will be not canceled. but by use critical section (in wide sense) we make this impossible. so possible only 2 orders: or

if (!_bQuit) ReadDirectoryChangesW(*); // (1)
_bQuit = true; CancelIoEx(*); // (2)

in this case ReadDirectoryChangesW will be canceled by CancelIoEx

or

_bQuit = true; CancelIoEx(*); // (1)
if (!_bQuit) ReadDirectoryChangesW(*); // (2)

in this case worked thread view _bQuit flag set and not call ReadDirectoryChangesW more.

in complete code this can look like:

inline ULONG BOOL_TO_ERROR(BOOL f)
{
    return f ? NOERROR : GetLastError();
}

struct WatchFolder : SRWLOCK
{
    HANDLE _hThread, _hFile;
    BOOLEAN _bQuit;

    WatchFolder() : _hThread(0), _hFile(0), _bQuit(false)
    {
        InitializeSRWLock(this);
    }

    ~WatchFolder()
    {
        if (_hThread) {
            WaitForSingleObject(_hThread, INFINITE);
            CloseHandle(_hThread);
        }
        if (_hFile) CloseHandle(_hFile);
    }

    static ULONG CALLBACK _WatchDirectory(PVOID This)
    {
        reinterpret_cast<WatchFolder*>(This)->WatchDirectory();
        return 0;
    }

    void WatchDirectory()
    {
        OVERLAPPED ov {};

        if (ov.hEvent = CreateEvent(0, 0, 0, 0))
        {
            union {
                FILE_NOTIFY_INFORMATION fni;
                char buf[0x800];// must be aligned as FILE_NOTIFY_INFORMATION
            };

            for(;;) 
            {
                AcquireSRWLockExclusive(this);

                ULONG dwError = _bQuit ? ERROR_OPERATION_ABORTED : BOOL_TO_ERROR(
                    ReadDirectoryChangesW(_hFile, buf, sizeof(buf), TRUE, FILE_NOTIFY_VALID_MASK, 0, &ov, 0));

                ReleaseSRWLockExclusive(this);

                ULONG NumberOfBytesTransferred = 0;

                if (dwError == NOERROR)
                {
                    dwError = BOOL_TO_ERROR(GetOverlappedResult(_hFile, &ov, &NumberOfBytesTransferred, TRUE));
                }

                if (dwError || !NumberOfBytesTransferred)
                {
                    if (dwError != ERROR_OPERATION_ABORTED)
                    {
                        __nop();
                    }
                    break;
                }

                FILE_NOTIFY_INFORMATION* pNotify = &fni;

                ULONG NextEntryOffset = 0;
                do 
                {
                    (PBYTE&)pNotify += NextEntryOffset;

                    DbgPrint("%x %.*S\n", pNotify->Action, pNotify->FileNameLength / sizeof(WCHAR), pNotify->FileName);

                } while (NextEntryOffset = pNotify->NextEntryOffset);
            }

            CloseHandle(ov.hEvent);
        }
    }

    ULONG Start(PCWSTR szFile)
    {
        HANDLE hFile = CreateFileW(szFile, FILE_GENERIC_READ, FILE_SHARE_VALID_FLAGS,
            NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED, NULL);

        ULONG dwError;

        if (hFile != INVALID_HANDLE_VALUE)
        {
            if (_hThread = CreateThread(0, 0, _WatchDirectory, this, 0, 0))
            {
                _hFile = hFile;

                return NOERROR;
            }
            dwError = GetLastError();
            CloseHandle(hFile);
        }
        else
        {
            dwError = GetLastError();
        }

        return dwError;
    }

    void Stop()
    {
        AcquireSRWLockExclusive(this);

        _bQuit = true, CancelIoEx(_hFile, 0);

        ReleaseSRWLockExclusive(this);
    }
};

void test()
{
    WatchFolder wf;
    if (wf.Start(L"somepath") == NOERROR)
    {
        MessageBoxW(0,0,0,0);
        wf.Stop();
    }
}

2) another way do this simply call CloseHandle(_hFile) instead CancelIoEx(_hFile, 0);. when handle(last, but assume you have only single handle) is closed - system end complete ReadDirectoryChangesW with status STATUS_NOTIFY_CLEANUP. code will be very similar to case CancelIoEx except now error on termination will be ERROR_NOTIFY_CLEANUP instead ERROR_OPERATION_ABORTED. but if use GetOverlappedResult[Ex] exist problem - this api have error in implementation - it lost all positive status values. it simply lost STATUS_NOTIFY_CLEANUP (but we of course can view it in Internal field of OVERLAPPED. code can be next:

            AcquireSRWLockExclusive(this);

            ULONG dwError = _bQuit ? ERROR_NOTIFY_CLEANUP : BOOL_TO_ERROR(
                ReadDirectoryChangesW(_hFile, buf, sizeof(buf), TRUE, FILE_NOTIFY_VALID_MASK, 0, &ov, 0));

            ReleaseSRWLockExclusive(this);

            ULONG NumberOfBytesTransferred = 0;

            if (dwError == NOERROR)
            {
                dwError = BOOL_TO_ERROR(GetOverlappedResult(_hFile, &ov, &NumberOfBytesTransferred, TRUE));
                // fix for error in GetOverlappedResult
                if (dwError == NOERROR && ov.Internal) dwError = RtlNtStatusToDosError((NTSTATUS)ov.Internal);
            }

            if (dwError || !NumberOfBytesTransferred)
            {
                if (dwError != ERROR_NOTIFY_CLEANUP)
                {
                    __nop();
                }
                break;
            }

and for stop

    AcquireSRWLockExclusive(this);

    _bQuit = true, CloseHandle(_hFile), _hFile = 0;

    ReleaseSRWLockExclusive(this);

3) else one option use alertable wait inside GetOverlappedResultEx and insert apc (or alert to worked thread). in this case we not need use critical section/or rundown-protection - because no matter will be apc (or alert) inserted before or after call ReadDirectoryChangesW - it anyway break wait.

            ULONG dwError = _bQuit ? STATUS_USER_APC : BOOL_TO_ERROR(
                ReadDirectoryChangesW(_hFile, buf, sizeof(buf), TRUE, FILE_NOTIFY_VALID_MASK, 0, &ov, 0));

            ULONG NumberOfBytesTransferred = 0;

            if (dwError == NOERROR)
            {
                dwError = BOOL_TO_ERROR(GetOverlappedResultEx(_hFile, &ov, &NumberOfBytesTransferred, INFINITE, TRUE));
            }

            if (dwError || !NumberOfBytesTransferred)
            {
                if (dwError == STATUS_USER_APC)
                {
                    CancelIo(_hFile);
                    GetOverlappedResult(_hFile, &ov, &NumberOfBytesTransferred, TRUE);
                }
                break;
            }

and for stop we need

static VOID NTAPI dummyAPC(_In_ ULONG_PTR )
{

}

_bQuit = true;
QueueUserAPC(dummyAPC, _hThread, 0);

of course instead call dummyAPC (which not need) better use alert, but GetOverlappedResultEx (more exactly WaitForSingleObjectEx) ignore STATUS_ALERT and again begin wait after it interrupted with STATUS_ALERT. so need use custom code here

ULONG
WINAPI
GetOverlappedResult2( _In_ LPOVERLAPPED lpOverlapped,
                      _Out_ PULONG_PTR lpNumberOfBytesTransferred)
{
    while (lpOverlapped->Internal == STATUS_PENDING)
    {
        if (NTSTATUS status = ZwWaitForSingleObject(lpOverlapped->hEvent, TRUE, 0))
        {
            return RtlNtStatusToDosError(status);
        }
    }

    KeMemoryBarrier();

    *lpNumberOfBytesTransferred = lpOverlapped->InternalHigh;

    return RtlNtStatusToDosError((NTSTATUS)lpOverlapped->Internal);
}

and with it can use next code:

            ULONG dwError = _bQuit ? ERROR_ALERTED : BOOL_TO_ERROR(
                ReadDirectoryChangesW(_hFile, buf, sizeof(buf), TRUE, FILE_NOTIFY_VALID_MASK, 0, &ov, 0));

            ULONG_PTR NumberOfBytesTransferred = 0;

            if (dwError == NOERROR)
            {
                dwError = GetOverlappedResult2(&ov, &NumberOfBytesTransferred);
            }

            if (dwError || !NumberOfBytesTransferred)
            {
                if (dwError == ERROR_ALERTED)
                {
                    CancelIo(_hFile);
                    GetOverlappedResult(_hFile, &ov, (ULONG*)&NumberOfBytesTransferred, TRUE);
                }
                break;
            }

and for stop

_bQuit = true;
NtAlertThread(_hThread);

4) however the best way for my option - not use dedicated thread all all, but use complete asynchronous I/O. example of code

struct WatchFolderCB : SRWLOCK, OVERLAPPED
{
    HANDLE _hFile;
    LONG _dwRefCount;
    union {
        FILE_NOTIFY_INFORMATION fni;
        char buf[0x800];// must be aligned as FILE_NOTIFY_INFORMATION
    };
    BOOLEAN _bQuit;

    void AddRef()
    {
        InterlockedIncrementNoFence(&_dwRefCount);
    }

    void Release()
    {
        if (!InterlockedDecrement(&_dwRefCount))
        {
            delete this;
        }
    }

    WatchFolderCB() : _hFile(0), _bQuit(false), _dwRefCount(1)
    {
        InitializeSRWLock(this);
        RtlZeroMemory(static_cast<OVERLAPPED*>(this), sizeof(OVERLAPPED));
    }

    ~WatchFolderCB()
    {
        if (_hFile) CloseHandle(_hFile);
    }

    static VOID WINAPI _IoCompletionCallback(
        _In_    DWORD dwErrorCode,
        _In_    DWORD dwNumberOfBytesTransfered,
        _Inout_ LPOVERLAPPED lpOverlapped
        )
    {
        static_cast<WatchFolderCB*>(lpOverlapped)->IoCompletionCallback(
            RtlNtStatusToDosError(dwErrorCode), dwNumberOfBytesTransfered);
    }

    VOID IoCompletionCallback(DWORD dwErrorCode, DWORD NumberOfBytesTransferred)
    {
        if (dwErrorCode || !NumberOfBytesTransferred)
        {
            if (dwErrorCode != ERROR_NOTIFY_CLEANUP)
            {
                __nop();
            }
        }
        else
        {
            FILE_NOTIFY_INFORMATION* pNotify = &fni;

            ULONG NextEntryOffset = 0;
            do 
            {
                (PBYTE&)pNotify += NextEntryOffset;

                DbgPrint("%x %.*S\n", pNotify->Action, pNotify->FileNameLength / sizeof(WCHAR), pNotify->FileName);

            } while (NextEntryOffset = pNotify->NextEntryOffset);

            ReadChanges();
        }

        Release();
    }

    void ReadChanges()
    {
        AddRef();

        AcquireSRWLockExclusive(this);

        ULONG dwError = _bQuit ? ERROR_NOTIFY_CLEANUP : BOOL_TO_ERROR(
            ReadDirectoryChangesW(_hFile, buf, sizeof(buf), TRUE, FILE_NOTIFY_VALID_MASK, 0, this, 0));

        ReleaseSRWLockExclusive(this);

        if (dwError)
        {
            IoCompletionCallback(dwError, 0);
        }
    }

    ULONG Start(PCWSTR szFile)
    {
        HANDLE hFile = CreateFileW(szFile, FILE_GENERIC_READ, FILE_SHARE_VALID_FLAGS,
            NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED, NULL);

        ULONG dwError;

        if (hFile != INVALID_HANDLE_VALUE)
        {
            if (BindIoCompletionCallback(hFile, _IoCompletionCallback, 0))
            {
                _hFile = hFile;
                ReadChanges();
                return NOERROR;
            }
            dwError = GetLastError();
            CloseHandle(hFile);
        }
        else
        {
            dwError = GetLastError();
        }

        return dwError;
    }

    void Stop()
    {
        AcquireSRWLockExclusive(this);

        _bQuit = true, CloseHandle(_hFile), _hFile = 0;

        ReleaseSRWLockExclusive(this);
    }
};


void test1()
{
    if (WatchFolderCB* p = new WatchFolderCB)
    {
        if (p->Start(L"*") == NOERROR)
        {
            MessageBoxW(0,0,0,0);
            p->Stop();
        }
        p->Release();
    }
}
RbMm
  • 31,280
  • 3
  • 35
  • 56
  • [Accessing inactive union member and undefined behavior?](https://stackoverflow.com/q/11373203/1889329) – IInspectable Apr 18 '20 at 21:46
  • 1
    @IInspectable no. my concrete code correct. also where you view accessing inactive union member - very interesting ? – RbMm Apr 18 '20 at 21:49
  • 1
    @IInspectable ok. if so hard formulate for you i explain where you mistake. take **adress** of object not mean access object. i take **address** of *fni* and formal documented that all unions members have the **same** address. so `&fni == &buf`. also we can pass `&fni` to `ReadDirectoryChangesW` instead `&buf` - no different - api take `PVOID` (in this case `fni` will be already "active" ? why ? we not access in code neither `fni` nor `buf`. really `buf` provides size of buffer here (because union must enough size for hold any member) and `fni` provide **align** here, because it required – RbMm Apr 18 '20 at 22:26
  • I understand what you are trying to accomplish. Yet, the code is not well defined. Whether you access a union member through a pointer or reference doesn't make a difference. Your compiler will make assumptions about the active member, and you are violating those assumptions. Needlessly, too. You don't need a `union` at all. C++ has the [alignas](https://en.cppreference.com/w/cpp/language/alignas) specifier to control alignment. – IInspectable Apr 18 '20 at 22:59
  • 1
    @IInspectable again - i not access any members of union. i not access `fni` (i not access any of it members) and i not access `buf` too (i not use `buf[i]` ) i use only **address** of `&fni==&buf` which is formal documented as the same. interesting - if i wrote `ReadDirectoryChangesW(_hFile, &fni, sizeof(buf)` (replace `buf` to `&fni` in api call) - now `fni` will be active ?! of if i do `pNotify = (FILE_NOTIFY_INFORMATION*)buf;` - this will be ok from your look ? and are `(FILE_NOTIFY_INFORMATION*)buf != &fni` ?? – RbMm Apr 18 '20 at 23:06
  • 1
    @IInspectable - really all this the same and only question of style - use typecast - `(FILE_NOTIFY_INFORMATION*)buf` or use union for get **adress** and align. or just allocate buffer like `(FILE_NOTIFY_INFORMATION*)new char[n]`. we can write `ReadDirectoryChangesW(hfile, &fni, sizeof(buf),` or can `ReadDirectoryChangesW(hfile, buf, sizeof(buf),` - both the **same**. which is better looking - another question. i be at all define second parameter of `ReadDirectoryChangesW` as `PFILE_NOTIFY_INFORMATION` instead of `PVOID` - this will be more correct, but ms define as define – RbMm Apr 18 '20 at 23:13
  • `&fni` accesses the `fni` member of the `union`, making it the active member. Accessing `&buf[0]` later accesses a member, that isn't the active member. There is no rule in the C++ specification that allows you to access a member in a union that is not the active union, just because they share the same address. This is just another shape of the strict aliasing rule. This isn't about style either. It is invalid, as written. You know how to fix it already. – IInspectable Apr 18 '20 at 23:16
  • 1
    @IInspectable - **no**. `&fni` - this is **not access** of `fni`. the `fni.Action` is **access**. like and `&buf` not access. when is `buf[0]` is access. also i ask several questions, but ok, narrow down to one - if i change code to `ReadDirectoryChangesW(hfile, &fni, sizeof(buf)` - it already will be correct from your look ? absolute concrete question. if not - what here yet not ok ? – RbMm Apr 18 '20 at 23:20
  • @riverofwind - i cannot help here, but i am writing a general answer from the principle – RbMm Apr 20 '20 at 20:37