Re: Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618”: Permission denied”

This is wrong, current code does well for this case. I should
broke the code during investigating the problem.

> > So, to make the windows version behave as the same,
> > dsm_impl_windows should return false if GetLastError() ==
> > ERROR_ALREADY_EXISTS regardless of hmap is valid. The current
> > behavior is wrong because two or more postmaster *share* the same
> > DSM segment instead of having their own segments.

> > The patch attached will fix *both of* the problems.

So, the patch fixes only the "Permission denied" case.

regards,

At Fri, 16 Oct 2015 14:37:47 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20151016(dot)143747(dot)121283630(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> > > Currently we are using error level as ERROR for creating dsm during
> > > postmaster startup which is not right and rather we should use error
> > > level as LOG. Can you please try with the attached patch and see
> > > if the issue is fixed for you.
> >
> > It seems somewhat strange. Looking into the create operation in
> > dms_impl_posix(), it should return false but not emit error when
> > the shared memory object already exists.
> >
> > So, to make the windows version behave as the same,
> > dsm_impl_windows should return false if GetLastError() ==
> > ERROR_ALREADY_EXISTS regardless of hmap is valid. The current
> > behavior is wrong because two or more postmaster *share* the same
> > DSM segment instead of having their own segments.
> >
> > On the other hand, in the case of GetLastError() ==
> > ERROR_ACCESS_DENIED, which occurs between postmasters exexuted as
> > service, it can reasonablly be assumed that the segment is
> > already created by someone else. I think this is no problem
> > because postgres processes don't need to access someone else's
> > segments.
> >
> > Finally, the valid fix would be that make it return false if
> > GetLastError() == ERROR_ALREADY_EXISTS or ERROR_ACCESS_DENIED.
> >
> > The patch attached will fix *both of* the problems.
> >
> >
> > regards,
> >
> >
> > At Fri, 16 Oct 2015 09:02:41 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in <CAA4eK1+pjtDouF-Lc9y0UgFDmWHnaf4KwiM1Y3Anq0wZ1gwsAA(at)mail(dot)gmail(dot)com>
> > > > I think that function dsm_impl_windows() with EACCES error should not
> > > > do ereport() with FATAL level. It works, but it is likely to make an
> > > > infinite loop if the user will receive EACCES error.
> > > >
> > > >
> > > Currently we are using error level as ERROR for creating dsm during
> > > postmaster startup which is not right and rather we should use error
> > > level as LOG. Can you please try with the attached patch and see
> > > if the issue is fixed for you.
> > >
> > > Another some what related point is currently we are using random()
> > > function to ensure a unique name for dsm and it seems to me that
> > > it is always going to generate same number on first invocation (at least
> > > thats what happening on windows) due to which you are seeing the
> > > error. Another options could be to append current pid or data directory
> > > path as we are doing in win32_shmem.c. I think this could be an
> > > optimization which can be done in addition to the fix attached (we can
> > > do this as a separate patch as well, if we agreed to do anything).
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center