On 2017-09-15 10:33:37 -0400, Tom Lane wrote: > Thomas Munro <thomas.mu...@enterprisedb.com> writes: > > Yeah. The regress tests in d36f7efb39e1b9613193b2e12717dbe2418ddae5 > > tested this stuff in parallel mode, but only a happy case to > > demonstrate blessed RECORD types travelling through tqueue.c correctly > > before and after ripping out the remapping stuff. The problem here > > was that I do some cleanup in the DSM detach hook of the new session > > segment, and that involved reading data that could point to another > > segment (if the DSA area needed more space). That can blow up if that > > other segment happens to have been unmapped first in the arbitrary > > order of resource cleanup in an aborting worker. I do have some > > thoughts on how to solve that general problem, but in this case it's > > completely unnecessary anyway. The attached patch fixes the problem > > by getting rid of the code that accesses shmem in the detach hook. > > With this patch applied installcheck survives on a cluster with > > force_parallel_worker=regress. > > I don't much like your proposed comment; the only way that this code > is even approximately correct is if we're exiting the process and > will never touch the RecordCacheArray again. (Otherwise, it risks > reassigning a previously used local typmod.)
How'd it reuse it after running the detach hook in workers? > My inclination is to just leave the local data structures alone. > There's nothing we can do to them that doesn't create more problems > than it solves, if the process continues to use the cache. The problem is that RecordCacheArray[n] can point into shared memory. Which means that the pushed change leaves around danging pointers into shared memory - unless I'm missing something (didn't yet have coffee, had to run to the store). Greetings, Andres Freund -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
