0

I have written a file system watcher for our project. Suddenly, it stopped getting events properly. I found out, that after GetOverlappedResult returns true, the result data are empty and so is bytes returned.

This is how I create file handle for watching a directory:

_directoryHandle = ::CreateFileA("some path", FILE_LIST_DIRECTORY, FILE_SHARE_READ | FILE_SHARE_DELETE | FILE_SHARE_WRITE,
  NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED, NULL);

This is how I start watching:

BOOL _watchRequestResult = false;
OVERLAPPED _ovl = { 0 };
static constexpr DWORD ResultDataSize = 20;
FILE_NOTIFY_INFORMATION _resultData[ResultDataSize] = { 0 };

_watchRequestResult = ::ReadDirectoryChangesW(
  _directoryHandle,
  (LPVOID)_resultData,
  ResultDataSize,
  TRUE,
  FILE_NOTIFY_CHANGE_FILE_NAME,
  NULL,
  &_ovl,
  NULL
);

After I use WaitForMultipleObjects to wait for the event (there's more than one), this is how I try to fetch the results:

DWORD _ovlBytesReturned;
if (::GetOverlappedResult(GetDirectoryHandle(), &_ovl, &_ovlBytesReturned, FALSE))
{
  // Read results
}

But suddenly when I copy file to watched directory the event fires - but I can see in the debugger that _ovlBytesReturned is 0 and _resultData also is just zeroes.

Is there any flag I could try changing to fix this? I'm quite sure it used to work, I have no idea what could have changed.

I already tried to change false to true in GetOverlappedResult(GetDirectoryHandle(), &_ovl, &_ovlBytesReturned, FALSE), in case there was additional need for waiting. It did not have any effect.

Tomáš Zato
  • 50,171
  • 52
  • 268
  • 778
  • 1
    *I use WaitForMultipleObjects* - you can not use it for wait result of `ReadDirectoryChangesW` because you not use any event inside `OVERLAPPED`. so your code already wrong – RbMm Oct 22 '19 at 16:52
  • The result cannot fit in a buffer that is only 20 bytes long. FILE_NOTIFY_INFORMATION is a variable-sized struct, you can't declare an array of them. – Hans Passant Oct 22 '19 at 16:52

1 Answers1

3

FILE_NOTIFY_INFORMATION is at least 16 bytes (for 0 wchar_ts long filenames) and you tell ReadDirectoryChangesW that you only have 20 bytes in the buffer (nBufferLength) - so overlapped results will have problems to fit. Use sizeof(_resultData) instead of ResultDataSize for the nBufferLength - but I think you should increase the size of the buffer a lot. 16*20 bytes isn't much when stuff starts happening.

Also note that you can't use _resultData[ index+1 ] to get to the next result. FILE_NOTIFY_INFORMATION is variable length, the next FILE_NOTIFY_INFORMATION is NextEntryOffset bytes ahead (with 0 meaning that you're at the last overlapped result).

You also need to create and assign an event handle (hEvent) in your OVERLAPPED structure in order for GetOverlappedResult() to work unless you use a completion routine instead - and the directory handle must be open all the time or you'll miss events.

Pseudo code:

handle = CreateFileW(...FILE_FLAG_OVERLAPPED...);
while(read_directory_changes) {
  ReadDirectoryChangesW();
  WaitForSingleObject() / WaitForMultipleObjects();
  GetOverlappedResult();
}
CloseHandle(handle);

Here's an example with those things in place.

#include <Windows.h>

#include <iomanip>
#include <iostream>
#include <memory>
#include <string>
#include <stdexcept>
#include <tuple>
#include <utility>
#include <vector>

// A base class for handles with different invalid values.
template<std::uintptr_t hInvalid>
class Handle {
public:
    Handle(const Handle&) = delete;
    Handle(Handle&& rhs) :
        hHandle(std::exchange(rhs.hHandle, hInvalid))
    {}
    Handle& operator=(const Handle&) = delete;
    Handle& operator=(Handle&& rhs) {
        std::swap(hHandle, rhs.hHandle);
        return *this;
    }

    // converting to a normal HANDLE
    operator HANDLE () { return hHandle; }

protected:
    Handle(HANDLE v) : hHandle(v) {
        // throw if we got an invalid handle
        if (hHandle == reinterpret_cast<HANDLE>(hInvalid))
            throw std::runtime_error("invalid handle");
    }
    ~Handle() {
        if (hHandle != reinterpret_cast<HANDLE>(hInvalid)) CloseHandle(hHandle);
    }
private:
    HANDLE hHandle;
};

using InvalidNullptrHandle = Handle<reinterpret_cast<std::uintptr_t>(nullptr)>;
using InvalidHandleValueHandle =
                Handle<reinterpret_cast<std::uintptr_t>(INVALID_HANDLE_VALUE)>;

// A class for directory handles
class DirectoryHandleW : public InvalidHandleValueHandle {
public:
    DirectoryHandleW(const std::wstring& dir) :
        Handle(
            ::CreateFileW(
                dir.c_str(), FILE_LIST_DIRECTORY,
                FILE_SHARE_READ | FILE_SHARE_DELETE | FILE_SHARE_WRITE,
                NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS |
                FILE_FLAG_OVERLAPPED, NULL)
        )
    {}
};

// A class for event handles
class EventHandle : public InvalidNullptrHandle {
public:
    EventHandle() :
        Handle(::CreateEvent(nullptr, true, false, nullptr))
    {}
};

// FILE_NOTIFY_INFORMATION action names
wchar_t const* get_action(DWORD a) {
    static wchar_t const* const Actions[FILE_ACTION_RENAMED_NEW_NAME + 1] = {
        L"Unknown action",
        L"ADDED",
        L"REMOVED",
        L"MODIFIED",
        L"RENAMED_OLD_NAME",
        L"RENAMED_NEW_NAME"
    };

    if (a > FILE_ACTION_RENAMED_NEW_NAME) a = 0;
    return Actions[a];
}

// A stepping function for FILE_NOTIFY_INFORMATION*
bool StepToNextNotifyInformation(FILE_NOTIFY_INFORMATION*& cur) {
    if (cur->NextEntryOffset == 0) return false;
    cur = reinterpret_cast<FILE_NOTIFY_INFORMATION*>(
        reinterpret_cast<char*>(cur) + cur->NextEntryOffset
    );
    return true;
}

// A ReadDirectoryChanges support class
template<size_t Handles=1, size_t BufByteSize = 4096>
class DirectoryChangesReader {
public:
    static_assert(Handles > 0, "There must be room for at least 1 HANDLE");
    static_assert(BufByteSize >= sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH, "BufByteSize too small");
    static_assert(BufByteSize % sizeof(DWORD) == 0, "BufByteSize must be a multiple of sizeof(DWORD)");

    DirectoryChangesReader(const std::wstring& dirname) :
        hDir(dirname),
        ovl{},
        hEv{},
        handles{hEv},
        buffer{std::make_unique<DWORD[]>(BufByteSize/sizeof(DWORD))}
    {}

    // A function to fill in data to use with ReadDirectoryChangesW
    void EnqueueReadDirectoryChanges() {
        ovl = OVERLAPPED{};
        ovl.hEvent = hEv;;
        BOOL rdc = ::ReadDirectoryChangesW(
            hDir,
            buffer.get(),
            BufByteSize,
            TRUE,
            FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_DIR_NAME |
            FILE_NOTIFY_CHANGE_ATTRIBUTES | FILE_NOTIFY_CHANGE_SIZE |
            FILE_NOTIFY_CHANGE_LAST_WRITE | FILE_NOTIFY_CHANGE_LAST_ACCESS |
            FILE_NOTIFY_CHANGE_CREATION | FILE_NOTIFY_CHANGE_SECURITY,
            NULL,
            &ovl,
            NULL
        );
        if (rdc == 0) throw std::runtime_error("EnqueueReadDirectoryChanges failed");
    }

    // A function to get a vector of <Action>, <Filename> pairs
    std::vector<std::pair<wchar_t const*, std::wstring>>
    GetDirectoryChangesResultW() {
        std::vector<std::pair<wchar_t const*, std::wstring>> retval;

        FILE_NOTIFY_INFORMATION* fni = reinterpret_cast<FILE_NOTIFY_INFORMATION*>(buffer.get());

        DWORD ovlBytesReturned;
        if (::GetOverlappedResult(hDir, &ovl, &ovlBytesReturned, TRUE)) {
            do {
                retval.emplace_back(
                    get_action(fni->Action),
                    std::wstring{fni->FileName,
                                 fni->FileName + fni->FileNameLength / sizeof(wchar_t)}
                );
            } while (StepToNextNotifyInformation(fni));
        }
        return retval;
    }

    // wait for the handles in the handles array
    DWORD WaitForHandles() {
        return ::WaitForMultipleObjects(Handles, handles, false, INFINITE);
    }

    // access to the handles array
    HANDLE& operator[](size_t idx) { return handles[idx]; }
    constexpr size_t handles_count() const { return Handles; }
private:
    DirectoryHandleW hDir;
    OVERLAPPED ovl;
    EventHandle hEv;
    HANDLE handles[Handles];
    std::unique_ptr<DWORD[]> buffer; // DWORD-aligned
};

int main()
{
    try {
        DirectoryChangesReader dcr(L"C:\\Users\\Ted\\Testing");

        while (true) {
            dcr.EnqueueReadDirectoryChanges();

            DWORD rv = dcr.WaitForHandles();
            if (rv == WAIT_OBJECT_0) {
                auto res = dcr.GetDirectoryChangesResultW();

                std::wcout << L"Got " << res.size() << L" changes\n";
                for (auto const& [action, filename] : res) {
                    std::wcout << action << L" " << filename << L"\n";
                }
            }
            else if (rv > WAIT_OBJECT_0 && rv < WAIT_OBJECT_0 + dcr.handles_count()) {
                // some other event you waited on
                auto event_idx = rv - WAIT_OBJECT_0;
            }
            else {
                std::wcerr << L"Some kind of problem\n";
                break;
            }
        }
    }
    catch (const std::exception& ex) {
        std::cout << ex.what() << "\n";
    }
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • @RbMm 4+4+4 + `wchar_t[1]` ... oh that's 16 ... right. But OP doesn't tell `ReadDirectoryChangesW` that the buffer is 16*20 bytes, only 20. Yeah, it may have other problems too, true. – Ted Lyngmo Oct 22 '19 at 17:01
  • 1
    yes, sorry, not view this at begin. really only `ResultDataSize` size. also i be note that OP can not *I use WaitForMultipleObjects to wait for the event* because no event in overlapped – RbMm Oct 22 '19 at 17:08
  • @RbMm Updated with what you spotted. Unfortunately I can't test it right now so OP may have more stuff missing. – Ted Lyngmo Oct 22 '19 at 17:23
  • *and GetOverlappedResult() should use that handle* not exactly, event inside `OVERLAPPED`, first parameter of `GetOverlappedResult` and must be formal file handle (not event) which used if I/O yet not complete, we want wait and no event inside overlapped. main use this event in `WaitForMultipleObjects` (i guess that GetDirectoryHandle() used here now), but i agree that direct error now from too small buffer – RbMm Oct 22 '19 at 17:29
  • Cheers @Remy :-) – Ted Lyngmo Oct 22 '19 at 18:30
  • 1
    Thank you for such a thought through answer. I actually do create an event in the overlapped structure, just forgot to mention it. But there§s not so much good resources on this problem on the internet, so I'm sure that the event init part will soon help somebody else. – Tomáš Zato Oct 23 '19 at 08:47
  • I asked [another question](https://stackoverflow.com/q/59884174/607407) since this still seems to not work perfectly. My code is basically this answer but I swaped the code so that `ReadDirectoryChangesW` is called again before processing the events. I'm missing events when multiple changes occur before I do the get overlapped result thing, because only one event is put in the result by the API. You can reproduce by adding some sleep into the processing. I used second processing thread, but the risk is still there, just smaller. – Tomáš Zato Jan 23 '20 at 17:38
  • @TedLyngmo Are you getting different messages in each of the pending `ReadDirectoryChanges`? I adapted mine code to also create two requests, but it's getting me the same updates, just twice. – Tomáš Zato Jan 24 '20 at 10:07
  • @TedLyngmo Hey, it took a few hours but I figured it out, mostly thanks to you! The thing is in my code, I'd close and re-open the directory handle for each watch event. Apparently, it's the dir handle that buffers the change events, somehow. When I changed my program to only have one directory handle per path, I started receiving events even when using single thread and sleeping 200ms after each received event. I tested with 40 files, mostly small ones. – Tomáš Zato Jan 24 '20 at 15:43
  • @TomášZato-ReinstateMonica I reworked this answer again. :-) – Ted Lyngmo Jan 25 '20 at 20:09