Re: BUG #13985: Segmentation fault on PREPARE TRANSACTION

From: Andres Freund <andres(at)anarazel(dot)de>
To: "Shulgin, Oleksandr" <oleksandr(dot)shulgin(at)zalando(dot)de>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, chris(dot)tessels(at)inergy(dot)nl, Pg Bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #13985: Segmentation fault on PREPARE TRANSACTION
Date: 2016-02-25 17:06:38
Message-ID: 20160225170638.z3yorrvkwpfuo2t5@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On 2016-02-25 09:02:07 +0100, Shulgin, Oleksandr wrote:
> On Wed, Feb 24, 2016 at 10:52 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> > On 2016-02-24 17:52:37 -0300, Alvaro Herrera wrote:
> > > chris(dot)tessels(at)inergy(dot)nl wrote:
> > >
> > > > Core was generated by `postgres: mailinfo_ow mailinfo_ods
> > 10.50.6.6(4188'.
> > > > Program terminated with signal 11, Segmentation fault.
> > > >
> > > > #0 MinimumActiveBackends (min=50) at procarray.c:2472
> > > > 2472 if (pgxact->xid == InvalidTransactionId)
> > >
> > > It's not surprising that you're not able to make this crash
> > > consistently, because it looks like the problem might be in concurrent
> > > modifications to the PGXACT array. This routine, MinimumActiveBackends,
> > > walks the PGPROC array explicitely without locks. There are comments
> > > indicating that this is safe, but evidently something has slipped in
> > > there.
> > >
> > > Apparently this code is trying to dereference an invalid pgxact, but
> > > it's not clear to me how this happens. Those structs are allocated in
> > > advance, and they are referenced in the code via array indexes, so even
> > > if the pgxact doesn't actually hold data about a valid transaction,
> > > dereferencing the XID shouldn't cause a crash.
> >
> > Well, that code is pretty, uh, questionable. E.g. for
> > int pgprocno =
> > arrayP->pgprocnos[index];
> > volatile PGPROC *proc = &allProcs[pgprocno];
> > volatile PGXACT *pgxact = &allPgXact[pgprocno];
> > there's no guarantee that pgprocno is actually the same index for both
> > lookups and the following
> > if (pgprocno == -1)
> > continue; /* do not count
> > deleted entries */
> > check. It's perfectly reasonable for a compiler to reload pgprocno from
> > memory, or just always reference it via memory.
> >
>
> Hm... this is against my understanding of what a compiler could (or
> should) do. Do you have a documentation reference or other evidence?

Which part does not conform to your expectations? Moving stores/loads
around from where they're apparently happening in the C program?
Repeatedly reading from memory instead of storing something on the
stack?

All of those are side effect free for single-threaded programs, which
pretty much is the gold standard for doing optimizations in C (at least
pre-C11, but even there the above all would be possible).

Regards,

Andres

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2016-02-25 17:20:06 Re: BUG #13985: Segmentation fault on PREPARE TRANSACTION
Previous Message Andres Freund 2016-02-25 17:03:11 Re: BUG #13985: Segmentation fault on PREPARE TRANSACTION