[PATCH/CFT] Fix setup.exe COM initialisation problem.

[Dave Korn-6]

[  I've got a few patches in the pipeline that I'll be able to send upstream
shortly.  This one needs some thorough testing, so I'm throwing it over the
wall early to give some advance notice and ask for help.  ]

  This fixes the "CoCreateInstance failed with error XXX, Setup will not be
able to create Cygwin Icons in the Start Menu or on the Desktop" problem on
win2k (and maybe earlier?).  I don't think the choice of threading model
matters much to setup.exe; we only use COM for shell functions and common
control dialogs, none of which we're doing in a parallelized or multi-threaded
fashion, so the serialisation between threads within setup.exe implied by
sending requests through an apartment shouldn't cause any noticeable slow
down; this stuff is all happening in a user-interaction-driven fashion anyway.

  However because of the scary comment about win7, I think this must be a
slightly hairy and not necessarily entirely backward-compatible area.  I've
tested it on XP and 2k; can anyone help out by checking it on any of
NT/2k3/Vista/2k8/W7 for me?  I just moved my desktop icons and start folder
menu out of the way, clicked through it in "install from local package dir"
and "keep" mode, and verified in windows explorer that it had created the new
shortcuts correctly, I think that's sufficient.

setup/ChangeLog:

        * main.cc (main_display): Use apartment- rather than multi-
        threaded COM threading model in CoInitialiseEx call.

Index: main.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/main.cc,v
retrieving revision 2.57
diff -p -u -r2.57 main.cc
--- main.cc 3 Oct 2009 10:25:37 -0000 2.57
+++ main.cc 23 Oct 2009 19:38:22 -0000
@@ -160,7 +160,7 @@ main_display ()
   // Windows 7 fails to create the ShellLink instance if this is
   // done later, in the thread which actually creates the shortcuts.
   extern IShellLink *sl;
-  CoInitializeEx (NULL, COINIT_MULTITHREADED);
+  CoInitializeEx (NULL, COINIT_APARTMENTTHREADED);
   HRESULT res = CoCreateInstance (&CLSID_ShellLink, NULL,
   CLSCTX_INPROC_SERVER, &IID_IShellLink,
   (LPVOID *) & sl);

    cheers,
      DaveK

[Dave Korn-6]

Dave Korn wrote:

>   I don't think the choice of threading model
> matters much to setup.exe; we only use COM for shell functions and common
> control dialogs, none of which we're doing in a parallelized or multi-threaded
> fashion, so the serialisation between threads within setup.exe implied by
> sending requests through an apartment shouldn't cause any noticeable slow
> down; this stuff is all happening in a user-interaction-driven fashion anyway.

  I've read a bit more about COM threading.  In mklink2.cc, it says:

> /* Initialized in WinMain.  This is required under Windows 7.  If
>    CoCreateInstance gets called from here, it fails to create the
>    instance with an undocumented error code 0x80110474.
>    FIXME: I have no idea why this happens. */

  Well, I have a theory.  Moving the CoCreate call into WinMain guarantees it
is being called by the same thread that called CoInitEx, and therefore has an
apartment allocated to it - whether single or multi isn't the point here.

  Now, when the CoCreate call was in make_link_2, it would be called from
whoever called DesktopSetupPage::OnFinish().

  That ought to be the main message loop, which ought to be the main thread,
which ought to be the same thread that called CoInitEx.  But if it was a
different thread, that had never called CoInitEx, the CoCreate call would fail.

  Is it possible that in win7 the message pump has been moved out of the main
thread, perhaps to try and solve or reduce the amount of desktop lockups and
thread deadlocks?

    cheers,
      DaveK

[Charles Wilson-2]

Dave Korn wrote:

>   However because of the scary comment about win7, I think this must be a
> slightly hairy and not necessarily entirely backward-compatible area.  I've
> tested it on XP and 2k; can anyone help out by checking it on any of
> NT/2k3/Vista/2k8/W7 for me?  I just moved my desktop icons and start folder
> menu out of the way, clicked through it in "install from local package dir"
> and "keep" mode, and verified in windows explorer that it had created the new
> shortcuts correctly, I think that's sufficient.

Windows Vista SP2: it did create the shortcuts.

However, on exit I got the "This program might not have installed
correctly...'Reinstall using recommended settings' or [confirm] 'This
program installed correctly'" dialog box.  Is that an indication of
crash-on-exit?  The log files seemed to be fine.

--
Chuck

[Dave Korn-6]

Charles Wilson wrote:

> Dave Korn wrote:
>
>>   However because of the scary comment about win7, I think this must be a
>> slightly hairy and not necessarily entirely backward-compatible area.  I've
>> tested it on XP and 2k; can anyone help out by checking it on any of
>> NT/2k3/Vista/2k8/W7 for me?  I just moved my desktop icons and start folder
>> menu out of the way, clicked through it in "install from local package dir"
>> and "keep" mode, and verified in windows explorer that it had created the new
>> shortcuts correctly, I think that's sufficient.
>
> Windows Vista SP2: it did create the shortcuts.
>
> However, on exit I got the "This program might not have installed
> correctly...'Reinstall using recommended settings' or [confirm] 'This
> program installed correctly'" dialog box.  Is that an indication of
> crash-on-exit?  The log files seemed to be fine.

  I think that's a UAC thingy to do with how elevated it was or was not and
manifests and whether it was trying to create All-Users or Current-User
shortcuts, probably?  I'm guessing that it's letting you know that it
redirected one of the shortcuts from one location to another and it's taking
that as a sign of an old non-limited-user-aware installer.

  Does it do anything different without the patch?  You might just not have
seen this before (or since the very first time you installed on that machine
which I don't know how long ago it was).

    cheers,
      DaveK

[Dave Korn-6]

Dave Korn wrote:
> Charles Wilson wrote:

>> Windows Vista SP2: it did create the shortcuts.
>>
>> However, on exit

>   I think that's a

  I forgot to say thanks!  Thanks for testing my patch Chuck!

    cheers,
      DaveK

[Charles Wilson-2]

Dave Korn wrote:

> Charles Wilson wrote:
>> However, on exit I got the "This program might not have installed
>> correctly...'Reinstall using recommended settings' or [confirm] 'This
>> program installed correctly'" dialog box.  Is that an indication of
>> crash-on-exit?  The log files seemed to be fine.
>
>   I think that's a UAC thingy to do with how elevated it was or was not and
> manifests and whether it was trying to create All-Users or Current-User
> shortcuts, probably?  I'm guessing that it's letting you know that it
> redirected one of the shortcuts from one location to another and it's taking
> that as a sign of an old non-limited-user-aware installer.

That could be. I was running as a regular user under UAC, installing for
"All Users" -- but I did allow the elevation.

>   Does it do anything different without the patch?

No, same behavior. So whatever it is, it's not caused by your patch.

> You might just not have
> seen this before (or since the very first time you installed on that machine
> which I don't know how long ago it was).

Normally I do not allow to create shortcuts (neither in the Start Menu
onr the Desktop) -- which is why I haven't seen this behavior before. I
manage my shortcuts manually using a "Toolbar Folder" added to my
QuickLaunch area: one for cygwin-1.5, and one for cygwin-1.7.

--
Chuck

[Corinna Vinschen-2]

On Oct 24 01:53, Dave Korn wrote:

> Charles Wilson wrote:
> > Dave Korn wrote:
> >
> >>   However because of the scary comment about win7, I think this must be a
> >> slightly hairy and not necessarily entirely backward-compatible area.  I've
> >> tested it on XP and 2k; can anyone help out by checking it on any of
> >> NT/2k3/Vista/2k8/W7 for me?  I just moved my desktop icons and start folder
> >> menu out of the way, clicked through it in "install from local package dir"
> >> and "keep" mode, and verified in windows explorer that it had created the new
> >> shortcuts correctly, I think that's sufficient.
> >
> > Windows Vista SP2: it did create the shortcuts.
> >
> > However, on exit I got the "This program might not have installed
> > correctly...'Reinstall using recommended settings' or [confirm] 'This
> > program installed correctly'" dialog box.  Is that an indication of
> > crash-on-exit?  The log files seemed to be fine.
>
>   I think that's a UAC thingy to do with how elevated it was or was not and

I tested setup with your patch applied on two Windows 7 32 and one 64 bit
machine and it worked fine in all three cases.  I didn't even get the
above "This program might not have installed correctly..." message, even
though I got it when using setup without your patch.  Looks like a clear
win to me.


Thanks,
Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

[Corinna Vinschen-2]

Hi Dave,

On Oct 24 11:44, Corinna Vinschen wrote:

> On Oct 24 01:53, Dave Korn wrote:
> > Charles Wilson wrote:
> > > Dave Korn wrote:
> > >>   However because of the scary comment about win7, I think this must be a
> > >> slightly hairy and not necessarily entirely backward-compatible area.  I've
> > >> tested it on XP and 2k; can anyone help out by checking it on any of
> > >> NT/2k3/Vista/2k8/W7 for me?
> > >> [...]
> > > Windows Vista SP2: it did create the shortcuts.
> > >
> > > However, on exit I got the "This program might not have installed
> > > correctly...'Reinstall using recommended settings' or [confirm] 'This
> > > program installed correctly'" dialog box.  Is that an indication of
> > > crash-on-exit?  The log files seemed to be fine.
> >
> >   I think that's a UAC thingy to do with how elevated it was or was not and
>
> I tested setup with your patch applied on two Windows 7 32 and one 64 bit
> machine and it worked fine in all three cases.  I didn't even get the
> above "This program might not have installed correctly..." message, even
> though I got it when using setup without your patch.  Looks like a clear
> win to me.

any reason that you didn't check this in already?


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

[Corinna Vinschen-2]

Dave?  Hello?

On Oct 30 15:59, Corinna Vinschen wrote:

> Hi Dave,
>
> On Oct 24 11:44, Corinna Vinschen wrote:
> > On Oct 24 01:53, Dave Korn wrote:
> > > Charles Wilson wrote:
> > > > Dave Korn wrote:
> > > >>   However because of the scary comment about win7, I think this must be a
> > > >> slightly hairy and not necessarily entirely backward-compatible area.  I've
> > > >> tested it on XP and 2k; can anyone help out by checking it on any of
> > > >> NT/2k3/Vista/2k8/W7 for me?
> > > >> [...]
> > > > Windows Vista SP2: it did create the shortcuts.
> > > >
> > > > However, on exit I got the "This program might not have installed
> > > > correctly...'Reinstall using recommended settings' or [confirm] 'This
> > > > program installed correctly'" dialog box.  Is that an indication of
> > > > crash-on-exit?  The log files seemed to be fine.
> > >
> > >   I think that's a UAC thingy to do with how elevated it was or was not and
> >
> > I tested setup with your patch applied on two Windows 7 32 and one 64 bit
> > machine and it worked fine in all three cases.  I didn't even get the
> > above "This program might not have installed correctly..." message, even
> > though I got it when using setup without your patch.  Looks like a clear
> > win to me.
>
> any reason that you didn't check this in already?

Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

[Dave Korn-6]

Corinna Vinschen wrote:

>
> any reason that you didn't check this in already?

  Been busy.  I'll get on with it, but could someone who has an NT4 system
please make a mental note to check setup.exe still works there sometime in the
not-too-distant, just in case?  TIA!

    cheers,
      DaveK

[Corinna Vinschen-2]

On Nov  4 15:31, Dave Korn wrote:
> Corinna Vinschen wrote:
>
> >
> > any reason that you didn't check this in already?
>
>   Been busy.  I'll get on with it, but could someone who has an NT4 system
> please make a mental note to check setup.exe still works there sometime in the
> not-too-distant, just in case?  TIA!

I can do that today or tomorrow.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

[Dave Korn-6]

Corinna Vinschen wrote:
> On Nov  4 15:31, Dave Korn wrote:
>> Corinna Vinschen wrote:
>>
>>> any reason that you didn't check this in already?
>>   Been busy.  I'll get on with it, but could someone who has an NT4 system
>> please make a mental note to check setup.exe still works there sometime in the
>> not-too-distant, just in case?  TIA!
>
> I can do that today or tomorrow.

  Thanks, hopefully should be no problem but COM is tricky and full of
historical quirks...

    cheers,
      DaveK

[Corinna Vinschen-2]

On Nov  4 15:55, Dave Korn wrote:

> Corinna Vinschen wrote:
> > On Nov  4 15:31, Dave Korn wrote:
> >> Corinna Vinschen wrote:
> >>
> >>> any reason that you didn't check this in already?
> >>   Been busy.  I'll get on with it, but could someone who has an NT4 system
> >> please make a mental note to check setup.exe still works there sometime in the
> >> not-too-distant, just in case?  TIA!
> >
> > I can do that today or tomorrow.
>
>   Thanks, hopefully should be no problem but COM is tricky and full of
> historical quirks...

Yup, still works on NT4.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat