bug#17510: 24.3.91; Problem with `emacs --daemon' in cygw32 build

[Ken Brown]

On 5/18/2014 11:11 AM, Eli Zaretskii wrote:
>> Date: Sun, 18 May 2014 10:30:28 -0400
>> From: Ken Brown <kbrown@cornell.edu>
>> CC: 17510@debbugs.gnu.org, dmantipov@yandex.ru
>>
>> On 5/18/2014 12:32 AM, Eli Zaretskii wrote:
>>> Thanks, but you need to be more selective: which one of these changes
>>> is the root cause, and why?
>>
>> It's not easy to be selective; these aren't independent changes.  There
>> used to be a `w32_display_name_list', which Dmitry removed.  Along with
>> this change, he removed the code that used to be in x_delete_display (to
>> delete a display from the list) and replaced it by a FIXME.
> 
> Perhaps that FIXME is not relevant to Cygwin, and the removed code
> should be retained in the Cygwin build.
> 
>>> In general, everything that is related to one_w32_display_info is
>>> specific to the WINDOWSNT port, so perhaps the problem is that the
>>> Cygwin-w32 build is incorrectly treated the same.  But where exactly?
>>
>> Looking at the code, I don't see why this problem is specific to the
>> Cygwin-w32 build.
> 
> If the Cygwin-w32 build wants more (or less) than one display_info
> object, then that part _is_ specific to Cygwin, because the native
> Windows build has only one such object that is never deleted.
> 
>> Can you reproduce it in the Windows build?
> 
> The native Windows build doesn't support --daemon, so no, I can't.
> 
>> I *think* what must be happening in the recipe that I gave for this bug
>> is that every time a client frame is closed, x_delete_display is called.
>>    Before Dmitry's change, this would actually delete something from a
>> list.  Now it doesn't, and the server gets messed up and ultimately dies
>> on the third attempt to create a client frame.
> 
> See above: try restoring that code for Cygwin only.
> 
>> Unless there's an obvious fix for this, it seems to me that we're far
>> enough into the pretest that we should just revert to the old code, at
>> least for emacs-24.
> 
> That would revert a useful cleanup, which I'm not sure is a good idea
> at this point.

How does this look?

=== modified file 'src/w32fns.c'
--- src/w32fns.c        2014-05-06 16:00:30 +0000
+++ src/w32fns.c        2014-05-18 19:21:41 +0000
@@ -5188,9 +5188,22 @@

   CHECK_STRING (name);

+#ifdef CYGWIN /* See comment in w32term.c */
+  Lisp_Object names;
+  for (dpyinfo = &one_w32_display_info, names = cygw32_display_name_list;
+       dpyinfo && !NILP (cygw32_display_name_list);
+       dpyinfo = dpyinfo->next, names = XCDR (names))
+    {
+      Lisp_Object tem;
+      tem = Fstring_equal (XCAR (XCAR (names)), name);
+      if (!NILP (tem))
+       return dpyinfo;
+    }
+#else  /* !CYGWIN */
   for (dpyinfo = &one_w32_display_info; dpyinfo; dpyinfo = dpyinfo->next)
     if (!NILP (Fstring_equal (XCAR (dpyinfo->name_list_element), name)))
       return dpyinfo;
+#endif /* !CYGWIN */

   /* Use this general default value to start with.  */
   Vx_resource_name = Vinvocation_name;
@@ -5326,10 +5339,17 @@
   (void)
 {
   Lisp_Object result = Qnil;
+
+#ifdef CYGWIN
+  Lisp_Object tail;
+  for (tail = cygw32_display_name_list; CONSP (tail); tail = XCDR (tail))
+    result = Fcons (XCAR (XCAR (tail)), result);
+#else
   struct w32_display_info *wdi;

   for (wdi = x_display_list; wdi; wdi = wdi->next)
     result = Fcons (XCAR (wdi->name_list_element), result);
+#endif

   return result;
 }

=== modified file 'src/w32term.c'
--- src/w32term.c       2014-04-16 14:00:39 +0000
+++ src/w32term.c       2014-05-18 19:24:17 +0000
@@ -100,6 +100,17 @@
 struct w32_display_info one_w32_display_info;
 struct w32_display_info *x_display_list;

+/* In order to support "emacs --daemon" on the Cygwin-w32 build, we
+   maintain a list of display names.  (This compensates for the fact
+   that the one display can't be deleted.  See Bug#17510 and the FIXME
+   in x_delete_display below.)  This is a list of cons cells, each of
+   the form (NAME . FONT-LIST-CACHE).  NAME is the name of the frame.
+   FONT-LIST-CACHE records previous values returned by
+   x-list-fonts.  */
+#ifdef CYGWIN
+Lisp_Object cygw32_display_name_list;
+#endif
+
 #if _WIN32_WINNT < 0x0500 && !defined(_W64)
 /* Pre Windows 2000, this was not available, but define it here so
    that Emacs compiled on such a platform will run on newer versions.
@@ -6162,6 +6173,10 @@
   memset (dpyinfo, 0, sizeof (*dpyinfo));

   dpyinfo->name_list_element = Fcons (display_name, Qnil);
+#ifdef CYGWIN
+  cygw32_display_name_list = Fcons (dpyinfo->name_list_element,
+                                   cygw32_display_name_list);
+#endif
   dpyinfo->w32_id_name = xmalloc (SCHARS (Vinvocation_name)
                                  + SCHARS (Vsystem_name) + 2);
   sprintf (dpyinfo->w32_id_name, "%s@%s",
@@ -6411,6 +6426,26 @@
 x_delete_display (struct w32_display_info *dpyinfo)
 {
   /* FIXME: the only display info apparently can't be deleted.  */
+#ifdef CYGWIN
+  if (! NILP (cygw32_display_name_list)
+      && EQ (XCAR (cygw32_display_name_list), dpyinfo->name_list_element))
+    cygw32_display_name_list = XCDR (cygw32_display_name_list);
+  else
+    {
+      Lisp_Object tail;
+
+      tail = cygw32_display_name_list;
+      while (CONSP (tail) && CONSP (XCDR (tail)))
+       {
+         if (EQ (XCAR (XCDR (tail)), dpyinfo->name_list_element))
+           {
+             XSETCDR (tail, XCDR (XCDR (tail)));
+             break;
+           }
+         tail = XCDR (tail);
+       }
+    }
+#endif /* CYGWIN */
   /* free palette table */
   {
     struct w32_palette_entry * plist;
@@ -6547,6 +6582,9 @@
 void
 syms_of_w32term (void)
 {
+  staticpro (&cygw32_display_name_list);
+  cygw32_display_name_list = Qnil;
+
   DEFSYM (Qvendor_specific_keysyms, "vendor-specific-keysyms");

   DEFSYM (Qadded, "added");

=== modified file 'src/w32term.h'
--- src/w32term.h       2014-02-04 16:13:51 +0000
+++ src/w32term.h       2014-05-18 19:13:35 +0000
@@ -200,6 +200,10 @@
 extern struct w32_display_info *x_display_list;
 extern struct w32_display_info one_w32_display_info;

+#ifdef CYGWIN
+extern Lisp_Object cygw32_display_name_list;
+#endif
+
 extern struct frame *x_window_to_frame (struct w32_display_info *, HWND);

 struct w32_display_info *x_display_info_for_name (Lisp_Object);