Re: Using glib's g_file_monitor_file and g_file_monitor_directory

[Eli Zaretskii]

> From: Michael Albinus <address@hidden>
> Cc: Ken Brown <address@hidden>,  Daniel Colascione <address@hidden>,  
> address@hidden,  address@hidden
> Date: Wed, 29 May 2013 17:57:19 +0200
> 
> Eli Zaretskii <address@hidden> writes:
> 
> >> I cannot test the MS Windows case; here I trust on you.
> >
> > Can you post an updated patch?
> 
> Appended.

Thanks, and sorry for the delay (I had a few other things on my
plate).

I tried this now; the testing was minimal and included a full
"configure && make" build, followed by invocation of auto-revert-mode
on some file.

First, I guess you tested this in a build that was linked against glib
regardless of gfilenotify.c.  Otherwise, I don't understand how it
compiled and linked for you under --with-file-notification=gfile,
because the compiler and linker flags reported by pkg-config for glib
were not propagated to src/Makefile.  The first 2 hunks below fix
that.  I don't think this is specific to w32, but feel free to point
out what I missed.

I also needed a few additional w32-specific changes to build without
any file notification mechanism; those changes are also below.

After that, both --with-file-notification=w32 and =no built fine, as
well as the default (without any switch), and they all seem to work as
expected.

When I used --with-file-notification=gfile, Emacs built OK, but
crashed in the bowels of glib the moment auto-revert-mode called the
glib monitor.  I post the backtrace below, for the record.  The glib
libraries I installed were stripped, so I couldn't step through their
code on the source level.  However, by comparing the disassembly with
the source code, I concluded that it crashes here:

  void
  _g_io_modules_ensure_extension_points_registered (void)
  {
    static gboolean registered_extensions = FALSE;
    GIOExtensionPoint *ep;

    G_LOCK (registered_extensions);   <<<<<<<<<<<<<<<<<<<<<<

G_LOCK has some convoluted expansion, of which there are several
variants, so I'm not sure which one exactly was used, but it is quite
clear that the crash was deliberate:

  (gdb) disassemble
  Dump of assembler code for function 
_g_io_modules_ensure_extension_points_registered:
     0x6d5a6044 <+0>:     push   %ebp
     0x6d5a6045 <+1>:     mov    %esp,%ebp
     0x6d5a6047 <+3>:     push   %esi
     0x6d5a6048 <+4>:     push   %ebx
     0x6d5a6049 <+5>:     sub    $0x10,%esp
     0x6d5a604c <+8>:     mov    0x6d64105c,%ebx
     0x6d5a6052 <+14>:    mov    0x4(%ebx),%esi
     0x6d5a6055 <+17>:    mov    0x6d632218,%eax
     0x6d5a605a <+22>:    test   %eax,%eax
     0x6d5a605c <+24>:    je     0x6d5a6158 
<_g_io_modules_ensure_extension_points_registered+276>
     0x6d5a6062 <+30>:    mov    %eax,(%esp)
     0x6d5a6065 <+33>:    call   *%esi
  => 0x6d5a6067 <+35>:    mov    0x6d63221c,%eax

The value of ESI is zero.

Here's the last portion of the backtrace; as you see, the file name is
perfectly alright:

  Program received signal SIGSEGV, Segmentation fault.
  0x00000000 in ?? ()
  (gdb) bt
  #0  0x00000000 in ?? ()
  #1  0x6d5a6067 in _g_io_modules_ensure_extension_points_registered ()
     from D:\usr\bin\libgio-2.0-0.dll
  #2  0x6d5a618e in _g_io_modules_ensure_loaded ()
     from D:\usr\bin\libgio-2.0-0.dll
  #3  0x6d5c2a58 in get_default_vfs () from D:\usr\bin\libgio-2.0-0.dll
  #4  0x6860d8bb in g_once_impl () from D:\usr\bin\libglib-2.0-0.dll
  #5  0x6d5c2d5c in g_vfs_get_default () from D:\usr\bin\libgio-2.0-0.dll
  #6  0x6d595d67 in g_file_new_for_path () from D:\usr\bin\libgio-2.0-0.dll
  #7  0x01291727 in Fgfile_add_watch (file=85666001, flags=58732934,
      callback=86315250) at gfilenotify.c:183
  #8  0x011f14dd in Ffuncall (nargs=4, args=0x88e414) at eval.c:2681

  Lisp Backtrace:
  "gfile-add-watch" (0x88e418)
  "byte-code" (0x88e67c)
  "auto-revert-notify-add-watch" (0x88eae8)
  "auto-revert-buffers" (0x88eedc)
  "apply" (0x88eed8)
  "byte-code" (0x88f13c)
  "timer-event-handler" (0x88f5ac)
  (gdb) frame 7
  #7  0x01291727 in Fgfile_add_watch (file=85666001, flags=58732934,
      callback=86315250) at gfilenotify.c:183
  183       gfile = g_file_new_for_path (SSDATA (ENCODE_FILE (file)));
  (gdb) p file
  $1 = 85666001
  (gdb) xtype
  Lisp_String
  (gdb) xstring
  $2 = (struct Lisp_String *) 0x51b28d0
  "d:/usr/eli/utils/emacs-trunk_2013-05-27/src"

So I guess we won't be recommending glib file notifications to Emacs
users on Windows just yet ;-)

Perhaps someone who knows more about glib on Windows will be able to
explain the crash and suggest a solution.

Finally, here are the changes I suggest on top of yours:

--- configure.ac~1      2013-06-02 07:32:48.220647900 +0300
+++ configure.ac        2013-06-02 09:57:31.173191700 +0300
@@ -2329,7 +2329,6 @@
    PKG_CHECK_MODULES(GFILENOTIFY, gio-2.0 >= 2.18, HAVE_GFILENOTIFY=yes, 
HAVE_GFILENOTIFY=no)
    if test "$HAVE_GFILENOTIFY" = "yes"; then
       AC_DEFINE(HAVE_GFILENOTIFY, 1, [Define to 1 if using GFile.])
-      LIBS="$LIBS $GFILENOTIFY_LIBS"
       NOTIFY_OBJ=gfilenotify.o
       NOTIFY_SUMMARY="yes -lgio (gfile)"
    fi
@@ -2359,6 +2358,8 @@
    AC_DEFINE(USE_FILE_NOTIFY, 1, [Define to 1 if using file notifications.])
 fi
 AC_SUBST(NOTIFY_OBJ)
+AC_SUBST(GFILENOTIFY_CFLAGS)
+AC_SUBST(GFILENOTIFY_LIBS)
 
 dnl Do not put whitespace before the #include statements below.
 dnl Older compilers (eg sunos4 cc) choke on it.


--- src/Makefile.in~1   2013-06-02 07:32:48.236247900 +0300
+++ src/Makefile.in     2013-06-02 09:56:15.342077800 +0300
@@ -160,6 +160,8 @@
 ## inotify.o if HAVE_INOTIFY.
 ## w32notify.o if HAVE_W32NOTIFY.
 NOTIFY_OBJ = @NOTIFY_OBJ@
+GFILENOTIFY_CFLAGS = @GFILENOTIFY_CFLAGS@
+GFILENOTIFY_LIBS = @GFILENOTIFY_LIBS@
 
 ## -ltermcap, or -lncurses, or -lcurses, or "".
 address@hidden@
@@ -342,7 +344,7 @@
   $(LIBXML2_CFLAGS) $(DBUS_CFLAGS) $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
-  $(LIBGNUTLS_CFLAGS) \
+  $(LIBGNUTLS_CFLAGS) $(GFILENOTIFY_CFLAGS) \
   $(WARN_CFLAGS) $(WERROR_CFLAGS) $(CFLAGS)
 ALL_OBJC_CFLAGS=$(ALL_CFLAGS) $(GNU_OBJC_CFLAGS)
 
@@ -424,7 +426,7 @@
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
    $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(LIB_PTHREAD_SIGMASK) \
-   $(LIB_MATH)
+   $(GFILENOTIFY_LIBS) $(LIB_MATH)
 
 all: emacs$(EXEEXT) $(OTHER_FILES)
 .PHONY: all


--- nt/config.nt~1      2013-05-09 07:10:33.000000000 +0300
+++ nt/config.nt        2013-06-02 07:49:56.258512600 +0300
@@ -547,6 +547,9 @@
 /* Define to 1 if you have the `gettimeofday' function. */
 #define HAVE_GETTIMEOFDAY 1
 
+/* Define to 1 if using GFile. */
+#undef HAVE_GFILENOTIFY
+
 /* Define to 1 if you have the `get_current_dir_name' function. */
 #undef HAVE_GET_CURRENT_DIR_NAME
 
@@ -1141,6 +1144,9 @@
 /* Define to 1 if you have the <vfork.h> header file. */
 #undef HAVE_VFORK_H
 
+/* Define to 1 to use w32notify. */
+#define HAVE_W32NOTIFY 1
+
 /* Define to 1 if you have the <wchar.h> header file. */
 #undef HAVE_WCHAR_H
 
@@ -1497,6 +1503,9 @@
 /* Define to nonzero if you want access control list support. */
 #undef USE_ACL
 
+/* Define to 1 if using file notifications. */
+#define USE_FILE_NOTIFY 1
+
 /* Define to 1 if using GTK. */
 #undef USE_GTK
 


--- src/w32inevt.c~1    2013-01-07 14:13:25.000000000 +0200
+++ src/w32inevt.c      2013-06-02 07:56:13.907896900 +0300
@@ -577,6 +577,7 @@
                     0, 0, 0);
 }
 
+#if HAVE_W32NOTIFY
 static int
 handle_file_notifications (struct input_event *hold_quit)
 {
@@ -644,6 +645,13 @@
   leave_crit ();
   return nevents;
 }
+#else  /* !HAVE_W32NOTIFY */
+static int
+handle_file_notifications (struct input_event *hold_quit)
+{
+  return 0;
+}
+#endif /* !HAVE_W32NOTIFY */
 
 /* Here's an overview of how Emacs input works in non-GUI sessions on
    MS-Windows.  (For description of the GUI input, see the commentary


--- src/w32term.c~1     2013-04-21 14:52:21.000000000 +0300
+++ src/w32term.c       2013-06-02 08:58:01.383496900 +0300
@@ -3210,6 +3210,8 @@
 }
 
 
+#if HAVE_W32NOTIFY
+
 /* File event notifications (see w32notify.c).  */
 
 Lisp_Object
@@ -3325,7 +3327,8 @@
   /* We've stuffed all the events ourselves, so w32_read_socket shouldn't.  */
   event->kind = NO_EVENT;
 }
-#endif
+#endif /* WINDOWSNT */
+#endif /* HAVE_W32NOTIFY */
 
 
 /* Function to report a mouse movement to the mainstream Emacs code.
@@ -4968,7 +4971,7 @@
          check_visibility = 1;
          break;
 
-#ifdef WINDOWSNT
+#if HAVE_W32NOTIFY
        case WM_EMACS_FILENOTIFY:
          f = x_window_to_frame (dpyinfo, msg.msg.hwnd);
          if (f)