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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 24 May 2014 15:59:07 +0300

> Date: Sat, 24 May 2014 08:38:14 -0400
> From: Ken Brown <[email-masked]>
> CC: [email-masked], [email-masked]
> 
> 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?

This looks OK to me, but I wonder: is it really correct not to free
w32_id_name at all?  And if that is correct, why only on Cygwin?

Does the Cygwin-w32 build also use a single dpyinfo object, like the
native Windows build?  If so, perhaps we need not free this in both
these builds.  IOW, I think your suggested change is OK for the
emacs-24 branch, but on the trunk I'd suggest to remove the xfree line
altogether.

Thanks.