Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows watch mode: fix a small race condition, re-enable tests #7190

Merged
merged 6 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/fswatch_win/fswatch_win.ml
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,16 @@ external wait : t -> sleep:int -> Event.t list = "fswatch_win_wait"

external add : t -> string -> unit = "fswatch_win_add"

let add t p =
if Filename.is_relative p then invalid_arg "Fswatch_win.add";
add t p

let wait t ~sleep =
List.filter
(function
| { Event.action = Modified; path; directory } -> (
try not (Sys.is_directory (Filename.concat directory path))
with Sys_error _ -> true)
with Sys_error _ -> (* should not happen *) true)
| _ -> true)
(wait t ~sleep)

Expand Down
20 changes: 18 additions & 2 deletions src/fswatch_win/fswatch_win_stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ struct fsenv {
HANDLE completionPort;
HANDLE signal;
HANDLE thread;
HANDLE afterAdd;
};

struct watch {
Expand Down Expand Up @@ -359,6 +360,7 @@ static DWORD WINAPI watch_thread(struct fsenv* fsenv) {
struct watch *w = add_watch(fsenv, key, &watches);
if (w != NULL && read_changes(w) == 0) remove_watch(w, &watches);
}
SetEvent(fsenv->afterAdd);
} else if (overlapped == (LPOVERLAPPED)SHUTDOWN) {
DEBUG("shutting down");
shuttingDown = TRUE;
Expand Down Expand Up @@ -399,15 +401,25 @@ value fswatch_win_create(value v_unit) {
unix_error("CreateEvent", err);
}

HANDLE afterAdd = CreateEvent(NULL, FALSE, FALSE, NULL);
if (afterAdd == NULL) {
DWORD err = GetLastError();
CloseHandle(signal);
CloseHandle(completionPort);
unix_error("CreateEvent", err);
}

struct fsenv* fsenv = caml_stat_alloc(sizeof(struct fsenv));
fsenv->events = NULL;
fsenv->completionPort = completionPort;
fsenv->signal = signal;
fsenv->afterAdd = afterAdd;

HANDLE thread = CreateThread(NULL, 0, &watch_thread, fsenv, 0, NULL);
if (thread == NULL) {
DWORD err = GetLastError();
caml_stat_free(fsenv);
CloseHandle(afterAdd);
CloseHandle(signal);
CloseHandle(completionPort);
unix_error("CreateThread", err);
Expand All @@ -433,6 +445,11 @@ value fswatch_win_add(value v_fsenv, value v_path) {
unix_error("PostQueuedCompletionStatus", GetLastError());
}

caml_release_runtime_system();
DWORD res = WaitForSingleObject(fsenv->afterAdd, INFINITE);
caml_acquire_runtime_system();
if (res != WAIT_OBJECT_0) unix_error("WaitForSingleObject", GetLastError());

return Val_unit;
}

Expand All @@ -443,10 +460,8 @@ value fswatch_win_wait(value v_fsenv, value v_debounce) {
caml_invalid_argument("Fswatch_win.wait: already shut down");

caml_release_runtime_system();

DWORD res = WaitForSingleObject(fsenv->signal, INFINITE);
if (res == WAIT_OBJECT_0 && Int_val(v_debounce) > 0) Sleep(Int_val(v_debounce));

caml_acquire_runtime_system();

if (res != WAIT_OBJECT_0)
Expand Down Expand Up @@ -480,6 +495,7 @@ value fswatch_win_shutdown(value v_fsenv) {
CloseHandle(fsenv->thread);
CloseHandle(fsenv->completionPort);
CloseHandle(fsenv->signal);
CloseHandle(fsenv->afterAdd);

for (struct events *e = pop_events(fsenv); e; e = e->next) {
free(e->path);
Expand Down
7 changes: 7 additions & 0 deletions test/unit-tests/fswatch_win/dune
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,10 @@
(sandbox always))
(action
(run ./fswatch_win_tests.exe)))

(alias
(name runtest)
(enabled_if
(= %{os_type} Win32))
(deps
(alias fswatch_win_tests)))
8 changes: 7 additions & 1 deletion test/unit-tests/fswatch_win/fswatch_win_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ let watch, collect_events =
create_file beginning_of_test_file;
create_file end_of_test_file;
let fswatch = Fswatch_win.create () in
let watch dir = Fswatch_win.add fswatch dir in
let watch dir =
let dir =
if Filename.is_relative dir then Filename.concat (Sys.getcwd ()) dir
else dir
in
Fswatch_win.add fswatch dir
in
watch markdir;
let rec collect_events acc = function
| [] ->
Expand Down