On 2017-09-15 10:33:37 -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@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
