Skip to content

Commit

Permalink
Hotreload overlapped fixes (#39)
Browse files Browse the repository at this point in the history
* Fix overlapped structure issues + add checks

* Fix incorrect recursion bool on fs watcher
Handle extra valid error case in linux fs watcher
  • Loading branch information
ccummingsNV authored Jul 12, 2024
1 parent ded39ca commit 126ffbd
Showing 1 changed file with 34 additions and 4 deletions.
38 changes: 34 additions & 4 deletions src/sgl/core/file_system_watcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
namespace sgl {

struct FileSystemWatchState {
FileSystemWatcher* watcher;
FileSystemWatchDesc desc;
#if SGL_WINDOWS
// NOTE: Windows relies on the OVERLAPPED structure being the first structure in the state.
OVERLAPPED overlapped;
HANDLE directory_handle;
char buffer[32 * 1024];
Expand All @@ -33,8 +32,13 @@ struct FileSystemWatchState {
#if !SGL_WINDOWS && !SGL_LINUX
std::map<std::filesystem::path, std::filesystem::file_time_type> files;
#endif
FileSystemWatcher* watcher;
FileSystemWatchDesc desc;
};

#if SGL_WINDOWS
static_assert(offsetof(FileSystemWatchState, overlapped) == 0);
#endif

#if SGL_WINDOWS
// Windows completion routine called with buffer of filesytem events.
Expand All @@ -52,7 +56,32 @@ FileIOCompletionRoutine(DWORD dwErrorCode, DWORD dwNumberOfBytesTransfered, LPOV

// Iterate over events, and call '_notify_change' on the watcher for each one.
do {

// This is an error that shouldn't happen on windows, but handling it just in case.
if ((offset + sizeof(FILE_NOTIFY_INFORMATION)) > dwNumberOfBytesTransfered) {
log_warn("Offset has invalid value and would overrun buffer: {}", offset);
break;
}

// Cast notify information to buffer
notify_information = (FILE_NOTIFY_INFORMATION*)((char*)state->buffer + offset);

// The strings in notify info follow it in the buffer, so use the 'NextEntryOffset'
// to check we won't go out of range reading from the buffer
if ((offset + notify_information->NextEntryOffset) > dwNumberOfBytesTransfered) {
log_warn("Next offset has invalid value and would overrun buffer: {}", offset);
break;
}

// Additional checks to catch string going out of range
size_t fn_offset = reinterpret_cast<char*>(notify_information->FileName) - state->buffer;
size_t fn_end = fn_offset + notify_information->FileNameLength;
if (fn_end > dwNumberOfBytesTransfered) {
log_warn("Filename would overrun buffer: {}", offset);
break;
}

// Now safely read string
std::wstring file_name(
notify_information->FileName,
notify_information->FileNameLength / sizeof(WCHAR)
Expand Down Expand Up @@ -82,14 +111,15 @@ FileIOCompletionRoutine(DWORD dwErrorCode, DWORD dwNumberOfBytesTransfered, LPOV
state->watcher->_notify_change(state, path, change);

offset += notify_information->NextEntryOffset;

} while (notify_information->NextEntryOffset != 0);

// Reissue the read request.
if (!ReadDirectoryChangesW(
state->directory_handle,
state->buffer,
sizeof(state->buffer),
TRUE,
FALSE,
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_SECURITY,
NULL,
Expand Down Expand Up @@ -366,7 +396,7 @@ void FileSystemWatcher::update()
}
offset += sizeof(struct inotify_event) + event->len;
}
} else if (errno != EAGAIN && errno != EWOULDBLOCK) {
} else if (errno != EAGAIN && errno != EWOULDBLOCK && errno != EBADF) {
SGL_THROW("Failed to read from inotify file descriptor");
}
#endif
Expand Down

0 comments on commit 126ffbd

Please sign in to comment.