At Mon, 06 Mar 2017 15:44:52 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote in <20170306.154452.254472341.horiguchi.kyot...@lab.ntt.co.jp> > Hello, > > At Sat, 4 Mar 2017 10:07:50 +0530, Amit Kapila <amit.kapil...@gmail.com> > wrote in <caa4ek1j0dczun00pwsiftvujpgfd2zq8cyxv9rytijpgbra...@mail.gmail.com> > > On Fri, Mar 3, 2017 at 3:49 PM, Kyotaro HORIGUCHI > > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > >> You can read about usage of LWLocks in extensions from below location: > > >> https://www.postgresql.org/docs/devel/static/xfunc-c.html#idp86986416 > > > > > > Thank you for the pointer. I understand that the document describes the > > > only > > > correct way to use LWLock in extensions and using LWLockRegisterTranche > > > is > > > a non-standard or prohibit way to do that. > > > > > > By the way, in the case of orafce, it uses LWLockRegisterTranche directly > > > but only when !found. So if any backend other than the creator of the > > > shmem > > > want to access tranche, the puch tranche is not found on the process and > > > crashes. I think this is it. > > > > > (I can't recall what the 'the puch' was...) > > > Yeah and I think that is expected if you use LWLockRegisterTranche. > > You might want to read comments in lwlock.h to know the reason behind > > the same. > > Ah, yes. I understood that. And even if a module prudently > registers its own tranche, it would be easily broken by some > other modules' omission to call LWLockNewTransactionId() for all > backends. As the result extension modules should not go that way.
It's wrong. The module knows the allocated tranche id so it can register the tranche properly. > > > If no other modules is installed, registeriing a tranche even if found > > > will > > > supress the crash but it is not a solution at all. > > > > > > At least for 9.6 or 10, orafce should do that following the documentation. > > > > > > > Agreed. > > > > > But it still can crash from the problem by the separate > > > NamedLWLockTrancheArray. (ID range check in LWLockInitialize would be > > > useless if it is not used by extensions) > > > > > > > What exact problem are you referring here? > > At many places in lwlock.c, especially on TRACE_POSTGRESQ_*() > macros, T_NAME(lock) is used and the macro just uses tranche id > as index of the main tranche array (LWLockTrancheArray). On the > other hand it is beyond the end of of the array for the "named > tranche"s and can raise SEGV. > > > I can guess two ways to fix this. One is change the definition of > T_NAME. > > | #define T_NAME(l) \ > | ((l)->tranche < LWTRANCHE_FIRST_USER_DEFINED ? \ > | LWLockTrancheArray[(l)->tranche] : \ > | NamedLWLockTrancheArray[(l)->tranche - LWTRANCHE_FIRST_USER_DEFINED] > > It makes the patch small but I don't thing the shape is > desirable. > > Then, the other way is registering named tranches into the main > tranche array. The number and names of the requested named > tranches are known to postmaster so they can be allocated and > initialized at the time. > > The mapping of the shared memory is inherited to backends so > pointing to the addresses in shared memory will work in the > !EXEC_BACKEND case. I confirmed that the behavior is ensured also > in EXEC_BACKEND case. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
