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

From: Ken Brown <[email-masked]>
Date: Sat, 24 May 2014 08:38:14 -0400

On 5/19/2014 3:25 PM, Ken Brown wrote:
> On 5/19/2014 12:46 PM, Eli Zaretskii wrote:
>> I guess it's OK for the branch, thanks.  But it strikes me that simply
>> replacing the car of dpyinfo->name_list_element by something like
>> "!!!DELETED DISPLAY!!!", or even just an empty string, would serve the
>> same purpose, and save us the nuisance of an additional list in
>> cygw32_display_name_list.  After all, all you need is to mark a
>> display deleted without actually deleting it, right?  IOW, the main
>> problem is in x_delete_display, and all the rest is just the overhead
>> you needed to fix that, correct?
>
> I think that's correct, and I agree that there should be a much simpler
> fix.  I'll have to look into the code and try to understand better
> exactly what happens when emacs is started as a daemon and then a client
> frame is opened and closed.

My guess as to the cause of this bug was completely wrong.  What happens 
in my recipe is that the pointer dpyinfo->w32_id_name is freed twice. 
(This is done in x_delete_display each time the only existing client 
frame is deleted.)  An attempt to create a client frame for the third 
time then leads to a crash because of malloc corruption.

I have no idea why this problem only showed up after Dmitry's code 
cleanup.  The only thing I can think of is that maintaining a list of 
display names, with insertions and deletions, masked the malloc corruption.

I think the right fix here would be to really delete the display when 
x_delete_display is called, free all resources, and set things up so 
that everything will be re-initialized if a new frame is created.  But 
this seems like a lot of trouble, possibly with unintended consequences. 
  The following is a much simpler workaround:

=== modified file 'src/w32term.c'
--- src/w32term.c       2014-04-16 14:00:39 +0000
+++ src/w32term.c       2014-05-24 12:13:15 +0000
@@ -6426,7 +6426,9 @@
      if (dpyinfo->palette)
        DeleteObject (dpyinfo->palette);
    }
+#ifndef CYGWIN
    xfree (dpyinfo->w32_id_name);
+#endif

    w32_reset_fringes ();
  }

I would of course add a comment explaining this.  What do you think?

Ken