On Wed, Jul 16, 2014 at 12:31 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Jul 16, 2014 at 2:11 PM, Magnus Hagander <magnus@hagander.net>
> wrote:
>> On Wed, Jul 16, 2014 at 6:37 AM, Amit Kapila <amit.kapila16@gmail.com>
>> wrote:
>>
>> There's also the change to throw an error if the source is already
>> registered, which is potentially a bigger problem.
>
> I think generally if the s/w is installed/registered and we try to install
> it
> second time without un-installing previous one, it gives some notice
> indicating the same.
>
>> Since the default
>> will be the same everywhere, do we really want to throw an error when
>> you install a second version, now that this is the normal state?
>
> Allowing second version to overwrite the first can also cause problems
> similar to what is reported in this thread, basically what if the second
> installation/registration is removed, won't it cause the first one to loose
> messages?
It will, this is true. I'm not sure there's a good way around that
given now Windows Event Logs work though, unless we restrict usage a
lot (as in only one installation of postgres at a time for example). I
thnk it's better to document what you need to do in a case like this
(re-register the existing DLL).
>> There's also definitely a problem in that that codepath fires up a
>> MessageBox, but it's just a function called in a DLL.
>
> There are other paths in the same code which already fires up
> MessageBox.
Ouch, I didn't realize that - I just looked at the patch. What's worse
is it's probably written by me :)
However, I'd say the one being added here is much more likely to fire
under such circumstances. Of the existing ones, the only really likely
case for them to fire is a permission denied case, and that will most
likely only happen from an interactive session. That might be why we
haven't seen it...
>> It might just as
>> well be called from a background service or from within an installer
>> with no visible desktop, at which point the process will appear to be
>> hung... I'm pretty sure you're not allowed to do that.
>
> So in that case shouldn't we get rid of MessageBox() or provide
> alternate way of logging from pgevent module.
It's something we might want to consider, but let's consider that a
separate patch.
Actually, it surprises me that we haven't had an issue before. But I
guess maybe the installers don't actually use regsvr32, but instead
just registers it manually by sticking it in the registry?
-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/