From f331970ae2cd3cde394f1172dd12b49ec807358b Mon Sep 17 00:00:00 2001 From: anth64 Date: Fri, 23 Jan 2026 23:39:24 +0100 Subject: [PATCH] refactor: optimize file watching and fix cross-platform reliability issues Refactor memory allocation patterns: - Replace realloc-in-loop with count-then-allocate pattern across all platforms - Eliminate arbitrary buffer sizes (e.g., malloc(8 * ...)) in favor of exact counts - Reduce allocation overhead by pre-counting items before malloc Fix Windows file watching: - Replace unreliable FindFirstChangeNotification with directory handle approach - Add is_file_ready() to prevent events while compiler is still writing files - Preserve timestamps when file is locked to retry on next poll - Fix do-while loop in platform_directory_init_scan (was skipping first file) Fix Linux inotify event handling: - Consolidate DELETE+CREATE pairs into single RELOAD event - Prevents duplicate events when compiler uses temp-file-and-rename pattern Fix BSD/macOS kqueue implementation: - Remove realloc loops from update_watches() and watch initialization - Pre-count files before allocating file descriptor arrays All platforms now correctly handle: - Compiler overwrites (temp file operations) - Manual copy/move operations - Explicit file deletions Tested on Linux, Windows 10, and FreeBSD. --- gmake.mk | 5 +- src/platform.c | 434 +++++++++++++++++++++++++++++++++---------------- 2 files changed, 294 insertions(+), 145 deletions(-) diff --git a/gmake.mk b/gmake.mk index 5bbc1c0..df88aff 100644 --- a/gmake.mk +++ b/gmake.mk @@ -1,13 +1,10 @@ include config.mk ifeq ($(OS),Windows_NT) - # Force the shell to cmd.exe to avoid bash/sh interference SHELL := cmd.exe FULL_LIB := $(LIB_NAME).dll LDFLAGS_PLAT := CFLAGS_PLAT := - # Windows-safe directory creation: check existence, then create - # Use 2>NUL to silence "directory already exists" warnings if any MKDIR = if not exist $(subst /,\,$(1)) mkdir $(subst /,\,$(1)) RMDIR = if exist $(subst /,\,$(1)) rd /s /q $(subst /,\,$(1)) else @@ -50,4 +47,4 @@ obj/release/%.o: src/%.c clean: @$(call RMDIR,$(OBJ_DIR)) - @$(call RMDIR,$(BIN_DIR)) \ No newline at end of file + @$(call RMDIR,$(BIN_DIR)) diff --git a/src/platform.c b/src/platform.c index 97e6f40..f5b1f43 100644 --- a/src/platform.c +++ b/src/platform.c @@ -28,6 +28,22 @@ int is_module_loaded(const char *filename, char (*loaded_ids)[STK_MOD_ID_BUFFER], size_t count); uint8_t is_valid_module_file(const char *filename); +#ifdef _WIN32 +static uint8_t is_file_ready(const char *dir_path, const char *filename) +{ + char full_path[STK_PATH_MAX_OS]; + HANDLE h; + + sprintf(full_path, "%s\\%s", dir_path, filename); + h = CreateFileA(full_path, GENERIC_READ, 0, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + if (h == INVALID_HANDLE_VALUE) + return 0; + CloseHandle(h); + return 1; +} +#endif + #ifndef __linux__ typedef struct { char filename[STK_PATH_MAX]; @@ -220,12 +236,13 @@ char (*platform_directory_init_scan(const char *dir_path, size_t *out_count)) if (h == INVALID_HANDLE_VALUE) goto exit; - while (FindNextFileA(h, &fd) && i < count) { + do { if ((fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) continue; - if (is_valid_module_file(fd.cFileName)) + if (is_valid_module_file(fd.cFileName) && i < count) strncpy(list[i++], fd.cFileName, STK_PATH_MAX - 1); - } + } while (FindNextFileA(h, &fd)); + FindClose(h); goto exit; @@ -300,13 +317,12 @@ static void update_watches(platform_watch_context_t *ctx) DIR *d; struct dirent *e; char f[STK_PATH_MAX_OS]; - size_t i; + size_t i, count = 0; int fd; - int *tmp_fds; + int *new_fds = NULL; - for (i = 0; i < ctx->watch.k.file_fd_count; i++) { + for (i = 0; i < ctx->watch.k.file_fd_count; i++) close(ctx->watch.k.file_fds[i]); - } free(ctx->watch.k.file_fds); ctx->watch.k.file_fds = NULL; ctx->watch.k.file_fd_count = 0; @@ -320,9 +336,28 @@ static void update_watches(platform_watch_context_t *ctx) if (!d) return; -scan_loop: +count_loop: e = readdir(d); if (!e) + goto count_done; + if (is_valid_module_file(e->d_name)) + count++; + goto count_loop; + +count_done: + if (count == 0) + goto cleanup; + + new_fds = malloc(count * sizeof(int)); + if (!new_fds) + goto cleanup; + + rewinddir(d); + i = 0; + +scan_loop: + e = readdir(d); + if (!e || i >= count) goto scan_done; if (!is_valid_module_file(e->d_name)) @@ -333,15 +368,7 @@ scan_loop: if (fd < 0) goto scan_loop; - tmp_fds = realloc(ctx->watch.k.file_fds, - sizeof(int) * (ctx->watch.k.file_fd_count + 1)); - if (!tmp_fds) { - close(fd); - goto scan_loop; - } - - ctx->watch.k.file_fds = tmp_fds; - ctx->watch.k.file_fds[ctx->watch.k.file_fd_count++] = fd; + new_fds[i++] = fd; EV_SET(&ev, fd, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_CLEAR, NOTE_WRITE | NOTE_ATTRIB, 0, NULL); @@ -350,13 +377,17 @@ scan_loop: goto scan_loop; scan_done: + ctx->watch.k.file_fds = new_fds; + ctx->watch.k.file_fd_count = i; + +cleanup: closedir(d); } #endif void *platform_directory_watch_start(const char *path) { -#if defined(__linux__) +#ifdef __linux__ int fd = inotify_init1(IN_NONBLOCK); if (fd < 0) return NULL; @@ -364,6 +395,18 @@ void *platform_directory_watch_start(const char *path) fd, path, IN_CLOSE_WRITE | IN_DELETE | IN_MOVED_TO | IN_MOVED_FROM); return (void *)(long)fd; #else +#ifdef _WIN32 + WIN32_FIND_DATAA fd; + HANDLE h; + char s[STK_PATH_MAX_OS]; + size_t count = 0, i = 0; +#else + DIR *d; + struct dirent *e; + struct stat st; + char f[STK_PATH_MAX_OS]; + size_t count = 0, i = 0; +#endif platform_watch_context_t *ctx = calloc(1, sizeof(platform_watch_context_t)); if (!ctx) @@ -371,85 +414,106 @@ void *platform_directory_watch_start(const char *path) strncpy(ctx->path, path, STK_PATH_MAX - 1); #ifdef _WIN32 - { - WIN32_FIND_DATAA fd; - HANDLE h; - char s[STK_PATH_MAX_OS]; - void *tmp_snaps; + ctx->watch.change_handle = + CreateFileA(path, FILE_LIST_DIRECTORY, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); - ctx->watch.change_handle = FindFirstChangeNotificationA( - path, FALSE, - FILE_NOTIFY_CHANGE_FILE_NAME | - FILE_NOTIFY_CHANGE_LAST_WRITE); + if (ctx->watch.change_handle == INVALID_HANDLE_VALUE) + goto done; - sprintf(s, "%s\\*", path); - h = FindFirstFileA(s, &fd); - if (h == INVALID_HANDLE_VALUE) - goto done; + sprintf(s, "%s\\*", path); + h = FindFirstFileA(s, &fd); + if (h == INVALID_HANDLE_VALUE) + goto done; - win_scan_loop: + do { if (!(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && - is_valid_module_file(fd.cFileName)) { - tmp_snaps = realloc(ctx->snaps, - (ctx->count + 1) * - sizeof(platform_snapshot_t)); - if (tmp_snaps) { - ctx->snaps = tmp_snaps; - strncpy(ctx->snaps[ctx->count].filename, - fd.cFileName, STK_PATH_MAX - 1); - ctx->snaps[ctx->count++].mtime = - fd.ftLastWriteTime; - } + is_valid_module_file(fd.cFileName)) + count++; + } while (FindNextFileA(h, &fd)); + + FindClose(h); + + if (count == 0) + goto done; + + ctx->snaps = malloc(count * sizeof(platform_snapshot_t)); + if (!ctx->snaps) + goto done; + + h = FindFirstFileA(s, &fd); + if (h == INVALID_HANDLE_VALUE) + goto done; + + do { + if (!(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && + is_valid_module_file(fd.cFileName) && i < count) { + strncpy(ctx->snaps[i].filename, fd.cFileName, + STK_PATH_MAX - 1); + ctx->snaps[i].mtime = fd.ftLastWriteTime; + i++; } - if (FindNextFileA(h, &fd)) - goto win_scan_loop; + } while (FindNextFileA(h, &fd)); + + FindClose(h); + ctx->count = i; - FindClose(h); - } #else - { - DIR *d; - struct dirent *e; - struct stat st; - char f[STK_PATH_MAX_OS]; - void *tmp_snaps; - ctx->watch.k.kq = kqueue(); - ctx->watch.k.dir_fd = open(path, O_RDONLY); + ctx->watch.k.kq = kqueue(); + ctx->watch.k.dir_fd = open(path, O_RDONLY); - d = opendir(path); - if (!d) - goto bsd_setup; + d = opendir(path); + if (!d) + goto bsd_setup; - bsd_scan_loop: - e = readdir(d); - if (!e) - goto bsd_scan_done; +bsd_count_loop: + e = readdir(d); + if (!e) + goto bsd_count_done; - sprintf(f, "%s/%s", path, e->d_name); - if (!is_valid_module_file(e->d_name)) - goto bsd_scan_loop; - if (stat(f, &st) != 0 || !S_ISREG(st.st_mode)) - goto bsd_scan_loop; + sprintf(f, "%s/%s", path, e->d_name); + if (!is_valid_module_file(e->d_name)) + goto bsd_count_loop; + if (stat(f, &st) != 0 || !S_ISREG(st.st_mode)) + goto bsd_count_loop; - tmp_snaps = realloc( - ctx->snaps, (ctx->count + 1) * sizeof(platform_snapshot_t)); - if (!tmp_snaps) - goto bsd_scan_loop; + count++; + goto bsd_count_loop; - ctx->snaps = tmp_snaps; - strncpy(ctx->snaps[ctx->count].filename, e->d_name, - STK_PATH_MAX - 1); - ctx->snaps[ctx->count++].mtime = st.st_mtime; +bsd_count_done: + if (count == 0) + goto bsd_setup; + ctx->snaps = malloc(count * sizeof(platform_snapshot_t)); + if (!ctx->snaps) + goto bsd_setup; + + rewinddir(d); + +bsd_scan_loop: + e = readdir(d); + if (!e || i >= count) + goto bsd_scan_done; + + sprintf(f, "%s/%s", path, e->d_name); + if (!is_valid_module_file(e->d_name)) + goto bsd_scan_loop; + if (stat(f, &st) != 0 || !S_ISREG(st.st_mode)) goto bsd_scan_loop; - bsd_scan_done: - closedir(d); + strncpy(ctx->snaps[i].filename, e->d_name, STK_PATH_MAX - 1); + ctx->snaps[i].mtime = st.st_mtime; + i++; + goto bsd_scan_loop; - bsd_setup: - update_watches(ctx); - } +bsd_scan_done: + closedir(d); + ctx->count = i; + +bsd_setup: + update_watches(ctx); #endif #ifdef _WIN32 @@ -473,11 +537,10 @@ void platform_directory_watch_stop(void *handle) return; #ifdef _WIN32 - FindCloseChangeNotification(ctx->watch.change_handle); + CloseHandle(ctx->watch.change_handle); #else - for (i = 0; i < ctx->watch.k.file_fd_count; i++) { + for (i = 0; i < ctx->watch.k.file_fd_count; i++) close(ctx->watch.k.file_fds[i]); - } free(ctx->watch.k.file_fds); close(ctx->watch.k.kq); close(ctx->watch.k.dir_fd); @@ -495,9 +558,9 @@ stk_module_event_t *platform_directory_watch_check( int fd = (int)(long)handle; char buf[STK_EVENT_BUFFER]; ssize_t len; - size_t idx = 0; + size_t idx = 0, count = 0; stk_module_event_t *evs; - char *ptr; + char *ptr, *end; struct inotify_event *e; int event_type; @@ -507,34 +570,74 @@ stk_module_event_t *platform_directory_watch_check( return NULL; } - evs = malloc(8 * sizeof(stk_module_event_t)); - *file_list = malloc(8 * sizeof(**file_list)); ptr = buf; - -process_inotify_loop: - if (ptr >= buf + len) - goto linux_done; - - e = (struct inotify_event *)ptr; - if (!e->len || !is_valid_module_file(e->name)) - goto next_event; - - strncpy((*file_list)[idx], e->name, STK_PATH_MAX - 1); - - event_type = STK_MOD_UNLOAD; - if (e->mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) { - if (is_module_loaded(e->name, loaded_ids, loaded_count) >= 0) - event_type = STK_MOD_RELOAD; - else - event_type = STK_MOD_LOAD; + end = buf + len; + while (ptr < end) { + e = (struct inotify_event *)ptr; + if (e->len && is_valid_module_file(e->name)) + count++; + ptr += sizeof(struct inotify_event) + e->len; } - evs[idx++] = event_type; -next_event: - ptr += sizeof(struct inotify_event) + e->len; - goto process_inotify_loop; + if (count == 0) { + *out_count = 0; + return NULL; + } + + evs = malloc(count * sizeof(stk_module_event_t)); + *file_list = malloc(count * sizeof(**file_list)); + if (!evs || !*file_list) { + free(evs); + free(*file_list); + *out_count = 0; + return NULL; + } + + ptr = buf; + while (ptr < end && idx < count) { + e = (struct inotify_event *)ptr; + if (e->len && is_valid_module_file(e->name)) { + if (e->mask & (IN_DELETE | IN_MOVED_FROM)) { + char *ptr2 = + ptr + sizeof(struct inotify_event) + e->len; + int has_create = 0; + + while (ptr2 < end) { + struct inotify_event *e2 = + (struct inotify_event *)ptr2; + if (e2->len && + strcmp(e->name, e2->name) == 0 && + (e2->mask & + (IN_CLOSE_WRITE | IN_MOVED_TO))) { + has_create = 1; + break; + } + ptr2 += sizeof(struct inotify_event) + + e2->len; + } + + if (has_create) { + ptr += sizeof(struct inotify_event) + + e->len; + continue; + } + } + + strncpy((*file_list)[idx], e->name, STK_PATH_MAX - 1); + + event_type = STK_MOD_UNLOAD; + if (e->mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) { + if (is_module_loaded(e->name, loaded_ids, + loaded_count) >= 0) + event_type = STK_MOD_RELOAD; + else + event_type = STK_MOD_LOAD; + } + evs[idx++] = event_type; + } + ptr += sizeof(struct inotify_event) + e->len; + } -linux_done: *out_count = idx; return evs; @@ -543,41 +646,52 @@ linux_done: platform_snapshot_t *new_snaps = NULL; size_t new_count = 0, i, j, ev_idx = 0; stk_module_event_t *evs = NULL; - void *tmp_ptr; int found; #ifdef _WIN32 WIN32_FIND_DATAA fd; HANDLE h; char s[STK_PATH_MAX_OS]; - - if (WaitForSingleObject(ctx->watch.change_handle, 0) != WAIT_OBJECT_0) - goto no_change; - - FindNextChangeNotification(ctx->watch.change_handle); + size_t count = 0; sprintf(s, "%s\\*", ctx->path); h = FindFirstFileA(s, &fd); if (h == INVALID_HANDLE_VALUE) goto build_diff; -win_snap_loop: - if (!(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && - is_valid_module_file(fd.cFileName)) { - tmp_ptr = realloc(new_snaps, (new_count + 1) * - sizeof(platform_snapshot_t)); - if (!tmp_ptr) - goto win_next; - new_snaps = tmp_ptr; - strncpy(new_snaps[new_count].filename, fd.cFileName, - STK_PATH_MAX - 1); - new_snaps[new_count++].mtime = fd.ftLastWriteTime; + do { + if (!(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && + is_valid_module_file(fd.cFileName)) + count++; + } while (FindNextFileA(h, &fd)); + + FindClose(h); + + if (count == 0) + goto build_diff; + + new_snaps = malloc(count * sizeof(platform_snapshot_t)); + if (!new_snaps) + goto build_diff; + + h = FindFirstFileA(s, &fd); + if (h == INVALID_HANDLE_VALUE) { + free(new_snaps); + new_snaps = NULL; + goto build_diff; } -win_next: - if (FindNextFileA(h, &fd)) - goto win_snap_loop; + + do { + if (!(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && + is_valid_module_file(fd.cFileName) && new_count < count) { + strncpy(new_snaps[new_count].filename, fd.cFileName, + STK_PATH_MAX - 1); + new_snaps[new_count].mtime = fd.ftLastWriteTime; + new_count++; + } + } while (FindNextFileA(h, &fd)); + FindClose(h); - goto build_diff; #else struct kevent kev; @@ -586,6 +700,7 @@ win_next: struct dirent *e; struct stat st; char f[STK_PATH_MAX_OS]; + size_t count = 0; if (kevent(ctx->watch.k.kq, NULL, 0, &kev, 1, &ts) <= 0) goto no_change; @@ -594,9 +709,38 @@ win_next: if (!d) goto bsd_update; -bsd_snap_loop: +bsd_count_loop: e = readdir(d); if (!e) + goto bsd_count_done; + + if (!is_valid_module_file(e->d_name)) + goto bsd_count_loop; + + sprintf(f, "%s/%s", ctx->path, e->d_name); + if (stat(f, &st) != 0 || !S_ISREG(st.st_mode)) + goto bsd_count_loop; + + count++; + goto bsd_count_loop; + +bsd_count_done: + if (count == 0) { + closedir(d); + goto bsd_update; + } + + new_snaps = malloc(count * sizeof(platform_snapshot_t)); + if (!new_snaps) { + closedir(d); + goto bsd_update; + } + + rewinddir(d); + +bsd_snap_loop: + e = readdir(d); + if (!e || new_count >= count) goto bsd_snap_done; if (!is_valid_module_file(e->d_name)) @@ -606,15 +750,9 @@ bsd_snap_loop: if (stat(f, &st) != 0 || !S_ISREG(st.st_mode)) goto bsd_snap_loop; - tmp_ptr = - realloc(new_snaps, (new_count + 1) * sizeof(platform_snapshot_t)); - if (!tmp_ptr) - goto bsd_snap_loop; - - new_snaps = tmp_ptr; strncpy(new_snaps[new_count].filename, e->d_name, STK_PATH_MAX - 1); - new_snaps[new_count++].mtime = st.st_mtime; - + new_snaps[new_count].mtime = st.st_mtime; + new_count++; goto bsd_snap_loop; bsd_snap_done: @@ -622,10 +760,11 @@ bsd_snap_done: bsd_update: update_watches(ctx); - goto build_diff; #endif +#ifdef _WIN32 build_diff: +#endif evs = malloc((ctx->count + new_count + 1) * sizeof(stk_module_event_t)); *file_list = malloc((ctx->count + new_count + 1) * sizeof(**file_list)); if (!evs || !*file_list) @@ -642,14 +781,25 @@ build_diff: #ifdef _WIN32 if (CompareFileTime(&ctx->snaps[i].mtime, &new_snaps[j].mtime) != 0) { + if (is_file_ready(ctx->path, + new_snaps[j].filename)) { + strncpy((*file_list)[ev_idx], + new_snaps[j].filename, + STK_PATH_MAX - 1); + evs[ev_idx++] = STK_MOD_RELOAD; + } else { + new_snaps[j].mtime = + ctx->snaps[i].mtime; + } + } #else if (ctx->snaps[i].mtime != new_snaps[j].mtime) { -#endif strncpy((*file_list)[ev_idx], new_snaps[j].filename, STK_PATH_MAX - 1); evs[ev_idx++] = STK_MOD_RELOAD; } +#endif break; } if (!found) { @@ -693,7 +843,9 @@ cleanup_error: cleanup_empty: free(new_snaps); +#ifndef _WIN32 no_change: +#endif *out_count = 0; return NULL; #endif