On Wed, Jul 16, 2014 at 12:31 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Wed, Jul 16, 2014 at 2:11 PM, Magnus Hagander <mag...@hagander.net> > wrote: >> On Wed, Jul 16, 2014 at 6:37 AM, Amit Kapila <amit.kapil...@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 Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers