Re: 8.3.5: Crash in CountActiveBackends() - lockless race? - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: 8.3.5: Crash in CountActiveBackends() - lockless race?
Date
Msg-id 49D0C48E.6030602@enterprisedb.com
Whole thread Raw
In response to 8.3.5: Crash in CountActiveBackends() - lockless race?  (Marko Kreen <markokr@gmail.com>)
Responses Re: 8.3.5: Crash in CountActiveBackends() - lockless race?  (Marko Kreen <markokr@gmail.com>)
List pgsql-hackers
Marko Kreen wrote:
> 1.  Add memory barrier to ProcArrayAdd/ProcArrayRemove between pointer
>     and count update.  This guarantees that partial slots will not be seen.
> 
> 2.  Always clear the pointer in ProcArrayRemove and check for NULL
>     in all "lockless" access points.  This guarantees that partial slots
>     will be either NULL or just-freed ones, before the barrier in
>     LWLockRelease(), which means the contents should be still sensible.
> 
> #1 seems to require platform-specific code, which we don't have yet?

Marking the pointer as volatile should work.

> So #2 may be easier solution.

Agreed. And more importantly, it puts the onus of getting it right into 
CountActiveBackends, which is the one who's breaking the rules. We don't 
necessarily need to clear the pointer in ProcArrayRemove either, the 
count doesn't need to be accurate.

Barring objections, I'll do #2:

*** procarray.c.~1.40.~    2008-01-09 23:52:36.000000000 +0200
--- procarray.c    2009-03-30 16:04:00.000000000 +0300
***************
*** 1088,1093 ****
--- 1088,1101 ----      for (index = 0; index < arrayP->numProcs; index++)      {          volatile PGPROC *proc =
arrayP->procs[index];
+         
+         /*
+          * Since we're not holding a lock, need to check that the pointer
+          * is valid. Someone holding the lock could have increased numProcs
+          * already, but not yet assigned a valid pointer to the array.
+          */
+         if (proc != NULL)
+             continue;
          if (proc == MyProc)              continue;            /* do not count myself */

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: More message encoding woes
Next
From: Gurjeet Singh
Date:
Subject: Re: New trigger option of pg_standby