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@gmail.com> wrote:
> 
>     On Wed, Sep 18, 2013 at 6:20 PM, Greg Stark <stark@mit.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@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +