Re: Dead code or buggy code?

On Fri, Sep 20, 2013 at 12:13:18AM +0100, Greg Stark wrote:
> So I'm just going to make the code defensive and assume NULL is possible when
> if maybe it isn't.
>
> In case it's not clear, this is one of the thing's Xi Wang's took picked up.
> There not to many but it turns out they are indeed not all in the adt code so I
> might wait until after the commit fest to commit it to avoid causing bit churn.

Uh, where are we on this?

---------------------------------------------------------------------------

>
> --
> greg
>
> On 19 Sep 2013 12:52, "Robert Haas" <robertmhaas(at)gmail(dot)com> wrote:
>
> On Wed, Sep 18, 2013 at 6:20 PM, Greg Stark <stark(at)mit(dot)edu> wrote:
> > The following code is in the ProcSleep at proc.c:1138.
> > GetBlockingAutoVacuumPgproc() should presumably always return a vacuum
> > pgproc entry since the deadlock state says it's blocked by autovacuum.
> > But I'm not really familiar enough with this codepath to know whether
> > there's not a race condition here where it can sometimes return null.
> > The following code checks autovac != NULL but the PGXACT initializer
> > would have seg faulted if it returned NULL if that's possible.
> >
> > if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM &&
> > allow_autovacuum_cancel)
> > {
> > PGPROC *autovac = GetBlockingAutoVacuumPgproc();
> > PGXACT *autovac_pgxact =
> > &ProcGlobal->allPgXact[autovac->pgprocno];
> >
> > LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
> >
> > /*
> > * Only do it if the worker is not working to protect against
> Xid
> > * wraparound.
> > */
> > if ((autovac != NULL) &&
> > (autovac_pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) &&
> > !(autovac_pgxact->vacuumFlags &
> PROC_VACUUM_FOR_WRAPAROUND))
> > {
>
> Hmm, yeah. I remember noticing this some time ago but never got
> around to fixing it. +1 for rearranging things there somehow.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +