From: Fabrice Popineau Date: Sat, 19 Mar 2016 12:44:53 +0000 (+0200) Subject: Improve w32notify notifications X-Git-Tag: emacs-26.0.90~2338 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=326fff41fa9f674d80be00b5c97c44f8043bbace;p=emacs.git Improve w32notify notifications * src/w32notify.c (DIRWATCH_BUFFER_SIZE): New macro. (struct notification): 'terminate' is now a HANDLE. (send_notifications): Argument is now a pointer to a notification. Don't loop waiting for the notification to be acknowledged by the main thread; instead, just add the notification to the linked list of notifications waiting to be acknowledged. (watch_end): Don't close the directory handle. (watch_completion): Allocate a new notification structure to be added to the notifications set. Call ReadDirectoryChangesW immediately after adding the new notification, and before sending a message to the main thread about them. (watch_worker): Don't loop calling ReadDirectoryChangesW; instead, call it just once -- it will be called again in watch_completion. Loop waiting for the main thread's indication to terminate. (start_watching): Create the event to be used to indicate to the worker thread that its should terminate. (remove_watch): Indicate to the worker thread that it should terminate. * src/w32term.c (queue_notifications): Loop over all the notifications in the linked list, processing all of them in one go. * src/w32inevt.c (handle_file_notifications): Loop over all the notifications in the linked list. * src/w32xfns.c (init_crit): Initialize the linked list of file notifications. (delete_crit): Free the linked list of file notifications, including any unprocessed notifications left in it. * src/w32term.h (struct notifications_se): New struct. * test/lisp/filenotify-tests.el (file-notify-test02-events) (file-notify-test05-dir-validity): Add read-event calls to facilitate event recognition by the main thread in batch mode. --- diff --git a/src/w32inevt.c b/src/w32inevt.c index a33f82dc7db..2269d318051 100644 --- a/src/w32inevt.c +++ b/src/w32inevt.c @@ -620,70 +620,89 @@ maybe_generate_resize_event (void) int handle_file_notifications (struct input_event *hold_quit) { - BYTE *p = file_notifications; - FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p; - const DWORD min_size - = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t); - struct input_event inev; + struct notifications_set *ns = NULL; int nevents = 0; + int done = 0; /* We cannot process notification before Emacs is fully initialized, since we need the UTF-16LE coding-system to be set up. */ if (!initialized) { - notification_buffer_in_use = 0; return nevents; } - enter_crit (); - if (notification_buffer_in_use) + while (!done) { - DWORD info_size = notifications_size; - Lisp_Object cs = Qutf_16le; - Lisp_Object obj = w32_get_watch_object (notifications_desc); - - /* notifications_size could be zero when the buffer of - notifications overflowed on the OS level, or when the - directory being watched was itself deleted. Do nothing in - that case. */ - if (info_size - && !NILP (obj) && CONSP (obj)) - { - Lisp_Object callback = XCDR (obj); + ns = NULL; - EVENT_INIT (inev); + /* Find out if there is a record available in the linked list of + notifications sets. If so, unlink te set from the linked list. + Use the critical section. */ + enter_crit (); + if (notifications_set_head->next != notifications_set_head) + { + ns = notifications_set_head->next; + ns->prev->next = ns->next; + ns->next->prev = ns->prev; + } + else + done = 1; + leave_crit(); - while (info_size >= min_size) + if (ns) + { + BYTE *p = ns->notifications; + FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p; + const DWORD min_size + = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t); + struct input_event inev; + DWORD info_size = ns->size; + Lisp_Object cs = Qutf_16le; + Lisp_Object obj = w32_get_watch_object (ns->desc); + + /* notifications size could be zero when the buffer of + notifications overflowed on the OS level, or when the + directory being watched was itself deleted. Do nothing in + that case. */ + if (info_size + && !NILP (obj) && CONSP (obj)) { - Lisp_Object utf_16_fn - = make_unibyte_string ((char *)fni->FileName, - fni->FileNameLength); - /* Note: mule-conf is preloaded, so utf-16le must - already be defined at this point. */ - Lisp_Object fname - = code_convert_string_norecord (utf_16_fn, cs, 0); - Lisp_Object action = lispy_file_action (fni->Action); - - inev.kind = FILE_NOTIFY_EVENT; - inev.timestamp = GetTickCount (); - inev.modifiers = 0; - inev.frame_or_window = callback; - inev.arg = Fcons (action, fname); - inev.arg = list3 (make_pointer_integer (notifications_desc), - action, fname); - kbd_buffer_store_event_hold (&inev, hold_quit); - nevents++; - - if (!fni->NextEntryOffset) - break; - p += fni->NextEntryOffset; - fni = (PFILE_NOTIFY_INFORMATION)p; - info_size -= fni->NextEntryOffset; + Lisp_Object callback = XCDR (obj); + + EVENT_INIT (inev); + + while (info_size >= min_size) + { + Lisp_Object utf_16_fn + = make_unibyte_string ((char *)fni->FileName, + fni->FileNameLength); + /* Note: mule-conf is preloaded, so utf-16le must + already be defined at this point. */ + Lisp_Object fname + = code_convert_string_norecord (utf_16_fn, cs, 0); + Lisp_Object action = lispy_file_action (fni->Action); + + inev.kind = FILE_NOTIFY_EVENT; + inev.timestamp = GetTickCount (); + inev.modifiers = 0; + inev.frame_or_window = callback; + inev.arg = Fcons (action, fname); + inev.arg = list3 (make_pointer_integer (ns->desc), + action, fname); + kbd_buffer_store_event_hold (&inev, hold_quit); + nevents++; + if (!fni->NextEntryOffset) + break; + p += fni->NextEntryOffset; + fni = (PFILE_NOTIFY_INFORMATION)p; + info_size -= fni->NextEntryOffset; + } } + /* Free this notification set. */ + free (ns->notifications); + free (ns); } - notification_buffer_in_use = 0; } - leave_crit (); return nevents; } #else /* !HAVE_W32NOTIFY */ diff --git a/src/w32notify.c b/src/w32notify.c index 586c2062f62..54d9bcc189a 100644 --- a/src/w32notify.c +++ b/src/w32notify.c @@ -22,27 +22,30 @@ along with GNU Emacs. If not, see . */ For each watch request, we launch a separate worker thread. The worker thread runs the watch_worker function, which issues an - asynchronous call to ReadDirectoryChangesW, and then waits in - SleepEx for that call to complete. Waiting in SleepEx puts the - thread in an "alertable" state, so it wakes up when either (a) the - call to ReadDirectoryChangesW completes, or (b) the main thread - instructs the worker thread to terminate by sending it an APC, see - below. + asynchronous call to ReadDirectoryChangesW, and then calls + WaitForSingleObjectEx to wait that an event be signaled + to terminate the thread. + Waiting with WaitForSingleObjectEx puts the thread in an + "alertable" state, so it wakes up when either (a) the call to + ReadDirectoryChangesW completes, or (b) the main thread instructs + the worker thread to terminate by signaling an event, see below. When the ReadDirectoryChangesW call completes, its completion routine watch_completion is automatically called. watch_completion - stashes the received file events in a buffer used to communicate - them to the main thread (using a critical section, so that several - threads could use the same buffer), posts a special message, - WM_EMACS_FILENOTIFY, to the Emacs's message queue, and returns. - That causes the SleepEx function call inside watch_worker to - return, and watch_worker then issues another call to - ReadDirectoryChangesW. (Except when it does not, see below.) + stashes the received file events in a linked list used to + communicate them to the main thread (using a critical section, so + that several threads could alter the same linked list), posts a + special message, WM_EMACS_FILENOTIFY, to the Emacs's message queue, + and returns. That causes the WaitForSingleObjectEx function call + inside watch_worker to return, but the thread won't terminate until + the event telling to do so will be signaled. The completion + routine issued another call to ReadDirectoryChangesW as quickly as + possible. (Except when it does not, see below.) In a GUI session, the WM_EMACS_FILENOTIFY message posted to the message queue gets dispatched to the main Emacs window procedure, which queues it for processing by w32_read_socket. When - w32_read_socket sees this message, it accesses the buffer with file + w32_read_socket sees this message, it accesses the linked list with file notifications (using a critical section), extracts the information, converts it to a series of FILE_NOTIFY_EVENT events, and stuffs them into the input event queue to be processed by keyboard.c input @@ -53,7 +56,7 @@ along with GNU Emacs. If not, see . */ procedures in console programs. That message wakes up MsgWaitForMultipleObjects inside sys_select, which then signals to its caller that some keyboard input is available. This causes - w32_console_read_socket to be called, which accesses the buffer + w32_console_read_socket to be called, which accesses the linked list with file notifications and stuffs them into the input event queue for keyboard.c to process. @@ -62,24 +65,21 @@ along with GNU Emacs. If not, see . */ bound to a command. The default binding is file-notify-handle-event, defined on subr.el. - After w32_read_socket or w32_console_read_socket are done - processing the notifications, they reset a flag signaling to all - watch worker threads that the notifications buffer is available for - more input. + Routines w32_read_socket or w32_console_read_socket process notifications + sets as long as some are available. When the watch is removed by a call to w32notify-rm-watch, the main - thread requests that the worker thread terminates by queuing an APC - for the worker thread. The APC specifies the watch_end function to - be called. watch_end calls CancelIo on the outstanding - ReadDirectoryChangesW call and closes the handle on which the - watched directory was open. When watch_end returns, the - watch_completion function is called one last time with the - ERROR_OPERATION_ABORTED status, which causes it to clean up and set - a flag telling watch_worker to exit without issuing another - ReadDirectoryChangesW call. Since watch_worker is the thread - procedure of the worker thread, exiting it causes the thread to - exit. The main thread waits for some time for the worker thread to - exit, and if it doesn't, terminates it forcibly. */ + thread requests that the worker thread terminates by signaling the + appropriate event and queuing an APC for the worker thread. The + APC specifies the watch_end function to be called. watch_end calls + CancelIo on the outstanding ReadDirectoryChangesW call. When + watch_end returns, the watch_completion function is called one last + time with the ERROR_OPERATION_ABORTED status, which causes it to + clean up and set a flag telling watch_worker to exit without + issuing another ReadDirectoryChangesW call. Since watch_worker is + the thread procedure of the worker thread, exiting it causes the + thread to exit. The main thread waits for some time for the worker + thread to exit, and if it doesn't, terminates it forcibly. */ #include #include @@ -98,6 +98,7 @@ along with GNU Emacs. If not, see . */ #include "frame.h" /* needed by termhooks.h */ #include "termhooks.h" /* for FILE_NOTIFY_EVENT */ +#define DIRWATCH_BUFFER_SIZE 16384 #define DIRWATCH_SIGNATURE 0x01233210 struct notification { @@ -108,73 +109,52 @@ struct notification { char *watchee; /* the file we are interested in, UTF-8 encoded */ HANDLE dir; /* handle to the watched directory */ HANDLE thr; /* handle to the thread that watches */ - volatile int terminate; /* if non-zero, request for the thread to terminate */ + HANDLE terminate; /* event signaling the thread to terminate */ unsigned signature; }; /* Used for communicating notifications to the main thread. */ -volatile int notification_buffer_in_use; -BYTE file_notifications[16384]; -DWORD notifications_size; -void *notifications_desc; +struct notifications_set *notifications_set_head; static Lisp_Object watch_list; /* Signal to the main thread that we have file notifications for it to process. */ static void -send_notifications (BYTE *info, DWORD info_size, void *desc, - volatile int *terminate) +send_notifications (struct notifications_set *ns) { int done = 0; struct frame *f = SELECTED_FRAME (); - /* A single buffer is used to communicate all notifications to the - main thread. Since both the main thread and several watcher - threads could be active at the same time, we use a critical area - and an "in-use" flag to synchronize them. A watcher thread can - only put its notifications in the buffer if it acquires the - critical area and finds the "in-use" flag reset. The main thread - resets the flag after it is done processing notifications. - - FIXME: is there a better way of dealing with this? */ - while (!done && !*terminate) - { + /* We add the current notification set to the linked list. Use the + critical section to make sure only one thread will access the + linked list. */ enter_crit (); - if (!notification_buffer_in_use) - { - if (info_size) - memcpy (file_notifications, info, - min (info_size, sizeof (file_notifications))); - notifications_size = min (info_size, sizeof (file_notifications)); - notifications_desc = desc; - /* If PostMessage fails, the message queue is full. If that - happens, the last thing they will worry about is file - notifications. So we effectively discard the - notification in that case. */ - if ((FRAME_TERMCAP_P (f) - /* We send the message to the main (a.k.a. "Lisp") - thread, where it will wake up MsgWaitForMultipleObjects - inside sys_select, causing it to report that there's - some keyboard input available. This will in turn cause - w32_console_read_socket to be called, which will pick - up the file notifications. */ - && PostThreadMessage (dwMainThreadId, WM_EMACS_FILENOTIFY, 0, 0)) - || (FRAME_W32_P (f) - && PostMessage (FRAME_W32_WINDOW (f), - WM_EMACS_FILENOTIFY, 0, 0)) - /* When we are running in batch mode, there's no one to - send a message, so we just signal the data is - available and hope sys_select will be called soon and - will read the data. */ - || (FRAME_INITIAL_P (f) && noninteractive)) - notification_buffer_in_use = 1; - done = 1; - } - leave_crit (); - if (!done) - Sleep (5); - } + ns->next = notifications_set_head; + ns->prev = notifications_set_head->prev; + ns->prev->next = ns; + notifications_set_head->prev = ns; + leave_crit(); + + /* If PostMessage fails, the message queue is full. If that + happens, the last thing they will worry about is file + notifications. So we effectively discard the notification in + that case. */ + if (FRAME_TERMCAP_P (f)) + /* We send the message to the main (a.k.a. "Lisp") thread, where + it will wake up MsgWaitForMultipleObjects inside sys_select, + causing it to report that there's some keyboard input + available. This will in turn cause w32_console_read_socket to + be called, which will pick up the file notifications. */ + PostThreadMessage (dwMainThreadId, WM_EMACS_FILENOTIFY, 0, 0); + else if (FRAME_W32_P (f)) + PostMessage (FRAME_W32_WINDOW (f), + WM_EMACS_FILENOTIFY, 0, 0); + /* When we are running in batch mode, there's no one to send a + message, so we just signal the data is available and hope + sys_select will be called soon and will read the data. */ + else if (FRAME_INITIAL_P (f) && noninteractive) + ; } /* An APC routine to cancel outstanding directory watch. Invoked by @@ -188,10 +168,7 @@ watch_end (ULONG_PTR arg) HANDLE hdir = (HANDLE)arg; if (hdir && hdir != INVALID_HANDLE_VALUE) - { - CancelIo (hdir); - CloseHandle (hdir); - } + CancelIo (hdir); } /* A completion routine (a.k.a. "APC function") for handling events @@ -202,13 +179,19 @@ VOID CALLBACK watch_completion (DWORD status, DWORD bytes_ret, OVERLAPPED *io_info) { struct notification *dirwatch; + DWORD _bytes; + struct notifications_set *ns = NULL; + BOOL terminate = FALSE; /* Who knows what happened? Perhaps the OVERLAPPED structure was freed by someone already? In any case, we cannot do anything with this request, so just punt and skip it. FIXME: should we raise the 'terminate' flag in this case? */ if (!io_info) - return; + { + DebPrint(("watch_completion: io_info is null.\n")); + return; + } /* We have a pointer to our dirwatch structure conveniently stashed away in the hEvent member of the OVERLAPPED struct. According to @@ -216,26 +199,69 @@ watch_completion (DWORD status, DWORD bytes_ret, OVERLAPPED *io_info) of the OVERLAPPED structure is not used by the system, so you can use it yourself." */ dirwatch = (struct notification *)io_info->hEvent; + if (status == ERROR_OPERATION_ABORTED) { /* We've been called because the main thread told us to issue CancelIo on the directory we watch, and watch_end did so. - The directory handle is already closed. We should clean up - and exit, signaling to the thread worker routine not to - issue another call to ReadDirectoryChangesW. Note that we - don't free the dirwatch object itself nor the memory consumed - by its buffers; this is done by the main thread in - remove_watch. Calling malloc/free from a thread other than - the main thread is a no-no. */ - dirwatch->dir = NULL; - dirwatch->terminate = 1; + We must exit, without issuing another call to + ReadDirectoryChangesW. */ + return; } - else + + /* We allocate a new set of notifications to be linked to the linked + list of notifications set. This will be processed by Emacs event + loop in the main thread. We need to duplicate the notifications + buffer, but not the dirwatch structure. */ + + /* Implementation note: In general, allocating memory in non-main + threads is a no-no in Emacs. We certainly cannot call xmalloc + and friends, because it can longjmp when allocation fails, which + will crash Emacs because the jmp_buf is set up to a location on + the main thread's stack. However, we can call 'malloc' directly, + since that is redirected to HeapAlloc that uses our private heap, + see w32heap.c, and that is thread-safe. */ + ns = malloc (sizeof(struct notifications_set)); + if (ns) + { + memset (ns, 0, sizeof(struct notifications_set)); + ns->notifications = malloc (bytes_ret); + if (ns->notifications) + { + memcpy (ns->notifications, dirwatch->buf, bytes_ret); + ns->size = bytes_ret; + ns->desc = dirwatch; + } + else + { + free (ns); + ns = NULL; + } + } + if (ns == NULL) + DebPrint(("Out of memory. Notifications lost.")); + + /* Calling ReadDirectoryChangesW quickly to watch again for new + notifications. */ + if (!ReadDirectoryChangesW (dirwatch->dir, dirwatch->buf, + DIRWATCH_BUFFER_SIZE, dirwatch->subtree, + dirwatch->filter, &_bytes, dirwatch->io_info, + watch_completion)) { - /* Tell the main thread we have notifications for it. */ - send_notifications (dirwatch->buf, bytes_ret, dirwatch, - &dirwatch->terminate); + DebPrint (("ReadDirectoryChangesW error: %lu\n", GetLastError ())); + /* If this call fails, it means that the directory is not + watchable any more. We need to terminate the worker thread. + Still, we will wait until the current notifications have been + sent to the main thread. */ + terminate = TRUE; } + + if (ns) + send_notifications(ns); + + /* If we were asked to terminate the thread, then fire the event. */ + if (terminate) + SetEvent(dirwatch->terminate); } /* Worker routine for the watch thread. */ @@ -243,42 +269,43 @@ static DWORD WINAPI watch_worker (LPVOID arg) { struct notification *dirwatch = (struct notification *)arg; + BOOL bErr; + DWORD _bytes = 0; + DWORD status; + + if (dirwatch->dir) + { + bErr = ReadDirectoryChangesW (dirwatch->dir, dirwatch->buf, + DIRWATCH_BUFFER_SIZE, dirwatch->subtree, + dirwatch->filter, &_bytes, + dirwatch->io_info, watch_completion); + if (!bErr) + { + DebPrint (("ReadDirectoryChangesW: %lu\n", GetLastError ())); + /* We cannot remove the dirwatch object from watch_list, + because we are in a separate thread. For the same + reason, we also cannot free memory consumed by the + buffers allocated for the dirwatch object. So we close + the directory handle, but do not free the object itself + or its buffers. We also don't touch the signature. This + way, remove_watch can still identify the object, remove + it, and free its memory. */ + CloseHandle (dirwatch->dir); + dirwatch->dir = NULL; + return 1; + } + } do { - BOOL status; - DWORD bytes_ret = 0; - - if (dirwatch->dir) - { - status = ReadDirectoryChangesW (dirwatch->dir, dirwatch->buf, 16384, - dirwatch->subtree, dirwatch->filter, - &bytes_ret, - dirwatch->io_info, watch_completion); - if (!status) - { - DebPrint (("watch_worker, abnormal exit: %lu\n", GetLastError ())); - /* We cannot remove the dirwatch object from watch_list, - because we are in a separate thread. For the same - reason, we also cannot free memory consumed by the - buffers allocated for the dirwatch object. So we close - the directory handle, but do not free the object itself - or its buffers. We also don't touch the signature. - This way, remove_watch can still identify the object, - remove it, and free its memory. */ - CloseHandle (dirwatch->dir); - dirwatch->dir = NULL; - return 1; - } - } - /* Sleep indefinitely until awoken by the I/O completion, which - could be either a change notification or a cancellation of the - watch. */ - SleepEx (INFINITE, TRUE); - } while (!dirwatch->terminate); + status = WaitForSingleObjectEx(dirwatch->terminate, INFINITE, TRUE); + } while (status == WAIT_IO_COMPLETION); + + /* The thread is about to terminate, so we clean up the dir handle. */ + CloseHandle (dirwatch->dir); + dirwatch->dir = NULL; return 0; } - /* Launch a thread to watch changes to FILE in a directory open on handle HDIR. */ static struct notification * @@ -287,7 +314,7 @@ start_watching (const char *file, HANDLE hdir, BOOL subdirs, DWORD flags) struct notification *dirwatch = xzalloc (sizeof (struct notification)); dirwatch->signature = DIRWATCH_SIGNATURE; - dirwatch->buf = xmalloc (16384); + dirwatch->buf = xmalloc (DIRWATCH_BUFFER_SIZE); dirwatch->io_info = xzalloc (sizeof(OVERLAPPED)); /* Stash a pointer to dirwatch structure for use by the completion routine. According to MSDN documentation of ReadDirectoryChangesW: @@ -297,7 +324,9 @@ start_watching (const char *file, HANDLE hdir, BOOL subdirs, DWORD flags) dirwatch->subtree = subdirs; dirwatch->filter = flags; dirwatch->watchee = xstrdup (file); - dirwatch->terminate = 0; + + dirwatch->terminate = CreateEvent(NULL, FALSE, FALSE, NULL); + dirwatch->dir = hdir; /* See w32proc.c where it calls CreateThread for the story behind @@ -307,11 +336,11 @@ start_watching (const char *file, HANDLE hdir, BOOL subdirs, DWORD flags) if (!dirwatch->thr) { + CloseHandle(dirwatch->terminate); xfree (dirwatch->buf); xfree (dirwatch->io_info); xfree (dirwatch->watchee); xfree (dirwatch); - dirwatch = NULL; } return dirwatch; } @@ -370,7 +399,10 @@ add_watch (const char *parent_dir, const char *file, BOOL subdirs, DWORD flags) return NULL; if ((dirwatch = start_watching (file, hdir, subdirs, flags)) == NULL) - CloseHandle (hdir); + { + CloseHandle (hdir); + dirwatch->dir = NULL; + } return dirwatch; } @@ -383,7 +415,7 @@ remove_watch (struct notification *dirwatch) { int i; BOOL status; - DWORD exit_code, err; + DWORD exit_code = 0, err; /* Only the thread that issued the outstanding I/O call can call CancelIo on it. (CancelIoEx is available only since Vista.) @@ -391,12 +423,10 @@ remove_watch (struct notification *dirwatch) to terminate. */ if (!QueueUserAPC (watch_end, dirwatch->thr, (ULONG_PTR)dirwatch->dir)) DebPrint (("QueueUserAPC failed (%lu)!\n", GetLastError ())); - /* We also set the terminate flag, for when the thread is - waiting on the critical section that never gets acquired. - FIXME: is there a cleaner method? Using SleepEx there is a - no-no, as that will lead to recursive APC invocations and - stack overflow. */ - dirwatch->terminate = 1; + + /* We also signal the thread that it can terminate. */ + SetEvent(dirwatch->terminate); + /* Wait for the thread to exit. FIXME: is there a better method that is not overly complex? */ for (i = 0; i < 50; i++) @@ -406,11 +436,13 @@ remove_watch (struct notification *dirwatch) break; Sleep (10); } + if ((status == FALSE && (err = GetLastError ()) == ERROR_INVALID_HANDLE) || exit_code == STILL_ACTIVE) { if (!(status == FALSE && err == ERROR_INVALID_HANDLE)) { + DebPrint(("Forcing thread termination.\n")); TerminateThread (dirwatch->thr, 0); if (dirwatch->dir) CloseHandle (dirwatch->dir); @@ -423,11 +455,11 @@ remove_watch (struct notification *dirwatch) CloseHandle (dirwatch->thr); dirwatch->thr = NULL; } + CloseHandle(dirwatch->terminate); xfree (dirwatch->buf); xfree (dirwatch->io_info); xfree (dirwatch->watchee); xfree (dirwatch); - return 0; } else diff --git a/src/w32term.c b/src/w32term.c index 62ad4eb086b..8955ce26b4b 100644 --- a/src/w32term.c +++ b/src/w32term.c @@ -3211,71 +3211,85 @@ static void queue_notifications (struct input_event *event, W32Msg *msg, struct frame *f, int *evcount) { - BYTE *p = file_notifications; - FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p; - const DWORD min_size - = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t); + struct notifications_set *ns = NULL; Lisp_Object frame; + int done = 0; /* We cannot process notification before Emacs is fully initialized, since we need the UTF-16LE coding-system to be set up. */ if (!initialized) - { - notification_buffer_in_use = 0; - return; - } + return; XSETFRAME (frame, f); - enter_crit (); - if (notification_buffer_in_use) + while (!done) { - DWORD info_size = notifications_size; - Lisp_Object cs = Qutf_16le; - Lisp_Object obj = w32_get_watch_object (notifications_desc); - - /* notifications_size could be zero when the buffer of - notifications overflowed on the OS level, or when the - directory being watched was itself deleted. Do nothing in - that case. */ - if (info_size - && !NILP (obj) && CONSP (obj)) + ns = NULL; + + /* Find out if there is a record available in the linked list of + notifications sets. If so, unlink the set from the linked + list. Use critical section. */ + enter_crit (); + if (notifications_set_head->next != notifications_set_head) { - Lisp_Object callback = XCDR (obj); + ns = notifications_set_head->next; + ns->prev->next = ns->next; + ns->next->prev = ns->prev; + } + else + done = 1; + leave_crit(); - while (info_size >= min_size) + if (ns) + { + BYTE *p = ns->notifications; + FILE_NOTIFY_INFORMATION *fni = (PFILE_NOTIFY_INFORMATION)p; + const DWORD min_size + = offsetof (FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t); + DWORD info_size = ns->size; + Lisp_Object cs = Qutf_16le; + Lisp_Object obj = w32_get_watch_object (ns->desc); + + /* notifications size could be zero when the buffer of + notifications overflowed on the OS level, or when the + directory being watched was itself deleted. Do nothing in + that case. */ + if (info_size + && !NILP (obj) && CONSP (obj)) { - Lisp_Object utf_16_fn - = make_unibyte_string ((char *)fni->FileName, - fni->FileNameLength); - /* Note: mule-conf is preloaded, so utf-16le must - already be defined at this point. */ - Lisp_Object fname - = code_convert_string_norecord (utf_16_fn, cs, 0); - Lisp_Object action = lispy_file_action (fni->Action); - - event->kind = FILE_NOTIFY_EVENT; - event->timestamp = msg->msg.time; - event->modifiers = 0; - event->frame_or_window = callback; - event->arg = list3 (make_pointer_integer (notifications_desc), - action, fname); - kbd_buffer_store_event (event); - (*evcount)++; - - if (!fni->NextEntryOffset) - break; - p += fni->NextEntryOffset; - fni = (PFILE_NOTIFY_INFORMATION)p; - info_size -= fni->NextEntryOffset; + Lisp_Object callback = XCDR (obj); + + while (info_size >= min_size) + { + Lisp_Object utf_16_fn + = make_unibyte_string ((char *)fni->FileName, + fni->FileNameLength); + /* Note: mule-conf is preloaded, so utf-16le must + already be defined at this point. */ + Lisp_Object fname + = code_convert_string_norecord (utf_16_fn, cs, 0); + Lisp_Object action = lispy_file_action (fni->Action); + + event->kind = FILE_NOTIFY_EVENT; + event->timestamp = msg->msg.time; + event->modifiers = 0; + event->frame_or_window = callback; + event->arg = list3 (make_pointer_integer (ns->desc), + action, fname); + kbd_buffer_store_event (event); + (*evcount)++; + if (!fni->NextEntryOffset) + break; + p += fni->NextEntryOffset; + fni = (PFILE_NOTIFY_INFORMATION)p; + info_size -= fni->NextEntryOffset; + } } + /* Free this notifications set. */ + xfree (ns->notifications); + xfree (ns); } - notification_buffer_in_use = 0; } - else - DebPrint (("We were promised notifications, but in-use flag is zero!\n")); - leave_crit (); - /* We've stuffed all the events ourselves, so w32_read_socket shouldn't. */ event->kind = NO_EVENT; } @@ -6949,6 +6963,8 @@ w32_init_main_thread (void) DuplicateHandle (GetCurrentProcess (), GetCurrentThread (), GetCurrentProcess (), &hMainThread, 0, TRUE, DUPLICATE_SAME_ACCESS); + + } DWORD WINAPI w32_msg_worker (void * arg); diff --git a/src/w32term.h b/src/w32term.h index d809be3c03d..8585c8190db 100644 --- a/src/w32term.h +++ b/src/w32term.h @@ -727,10 +727,18 @@ extern void x_delete_display (struct w32_display_info *dpyinfo); extern void x_query_color (struct frame *, XColor *); -extern volatile int notification_buffer_in_use; -extern BYTE file_notifications[16384]; -extern DWORD notifications_size; -extern void *notifications_desc; +#define FILE_NOTIFICATIONS_SIZE 16384 +/* Notifications come in sets. We use a doubly linked list with a + sentinel to communicate those sets from the watching threads to the + main thread. */ +struct notifications_set { + LPBYTE notifications; + DWORD size; + void *desc; + struct notifications_set *next; + struct notifications_set *prev; +}; +extern struct notifications_set *notifications_set_head; extern Lisp_Object w32_get_watch_object (void *); extern Lisp_Object lispy_file_action (DWORD); extern int handle_file_notifications (struct input_event *); diff --git a/src/w32xfns.c b/src/w32xfns.c index 04bf5ce733e..9b633c4c56a 100644 --- a/src/w32xfns.c +++ b/src/w32xfns.c @@ -48,6 +48,19 @@ init_crit (void) when the input queue is empty, so make it a manual reset event. */ input_available = CreateEvent (NULL, TRUE, FALSE, NULL); + /* Initialize the linked list of notifications sets that will be + used to communicate between the watching worker threads and the + main thread. */ + notifications_set_head = malloc (sizeof(struct notifications_set)); + if (notifications_set_head) + { + memset (notifications_set_head, 0, sizeof(struct notifications_set)); + notifications_set_head->next + = notifications_set_head->prev = notifications_set_head; + } + else + DebPrint(("Out of memory: can't initialize notifications sets.")); + #ifdef WINDOWSNT keyboard_handle = input_available; #endif /* WINDOWSNT */ @@ -76,6 +89,21 @@ delete_crit (void) CloseHandle (interrupt_handle); interrupt_handle = NULL; } + + if (notifications_set_head) + { + /* Free any remaining notifications set that could be left over. */ + while (notifications_set_head->next != notifications_set_head) + { + struct notifications_set *ns = notifications_set_head->next; + notifications_set_head->next = ns->next; + ns->next->prev = notifications_set_head; + if (ns->notifications) + free (ns->notifications); + free (ns); + } + } + free (notifications_set_head); } void diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el index d3610f00080..39088949b36 100644 --- a/test/lisp/filenotify-tests.el +++ b/test/lisp/filenotify-tests.el @@ -433,7 +433,8 @@ longer than timeout seconds for the events to be delivered." (write-region "any text" nil file-notify--test-tmpfile nil 'no-message) (read-event nil nil file-notify--test-read-event-timeout) - (delete-directory temporary-file-directory 'recursive)) + (delete-directory temporary-file-directory 'recursive) + (read-event nil nil file-notify--test-read-event-timeout)) (file-notify-rm-watch file-notify--test-desc)) ;; Check copy of files inside a directory. @@ -475,7 +476,8 @@ longer than timeout seconds for the events to be delivered." (read-event nil nil file-notify--test-read-event-timeout) (set-file-times file-notify--test-tmpfile '(0 0)) (read-event nil nil file-notify--test-read-event-timeout) - (delete-directory temporary-file-directory 'recursive)) + (delete-directory temporary-file-directory 'recursive) + (read-event nil nil file-notify--test-read-event-timeout)) (file-notify-rm-watch file-notify--test-desc)) ;; Check rename of files inside a directory. @@ -509,7 +511,8 @@ longer than timeout seconds for the events to be delivered." (rename-file file-notify--test-tmpfile file-notify--test-tmpfile1) ;; After the rename, we won't get events anymore. (read-event nil nil file-notify--test-read-event-timeout) - (delete-directory temporary-file-directory 'recursive)) + (delete-directory temporary-file-directory 'recursive) + (read-event nil nil file-notify--test-read-event-timeout)) (file-notify-rm-watch file-notify--test-desc)) ;; Check attribute change. Does not work for cygwin. @@ -527,7 +530,7 @@ longer than timeout seconds for the events to be delivered." ;; w32notify does not distinguish between `changed' and ;; `attribute-changed'. ((string-equal (file-notify--test-library) "w32notify") - '(changed changed changed changed)) + '(changed changed)) ;; For kqueue and in the remote case, `write-region' ;; raises also an `attribute-changed' event. ((or (string-equal (file-notify--test-library) "kqueue") @@ -754,7 +757,9 @@ longer than timeout seconds for the events to be delivered." (should (file-notify-valid-p file-notify--test-desc)) ;; After removing the watch, the descriptor must not be valid ;; anymore. + (read-event nil nil file-notify--test-read-event-timeout) (file-notify-rm-watch file-notify--test-desc) + (read-event nil nil file-notify--test-read-event-timeout) (file-notify--wait-for-events (file-notify--test-timeout) (not (file-notify-valid-p file-notify--test-desc))) @@ -776,7 +781,9 @@ longer than timeout seconds for the events to be delivered." (should (file-notify-valid-p file-notify--test-desc)) ;; After deleting the directory, the descriptor must not be ;; valid anymore. + (read-event nil nil file-notify--test-read-event-timeout) (delete-directory file-notify--test-tmpfile t) + (read-event nil nil file-notify--test-read-event-timeout) (file-notify--wait-for-events (file-notify--test-timeout) (not (file-notify-valid-p file-notify--test-desc)))