On 3 September 2012 08:10, Daniel Farina <daniel@heroku.com> wrote:
> http://doxygen.postgresql.org/xlog_8c_source.html#l08197
>
> On line 8197 of xlog.c:
>
> 08194 /* Get a local copy of the last safe checkpoint record. */
> 08195 SpinLockAcquire(&xlogctl->info_lck);
> 08196 lastCheckPointRecPtr = xlogctl->lastCheckPointRecPtr;
> 08197 memcpy(&lastCheckPoint, &XLogCtl->lastCheckPoint, sizeof(CheckPoint));
> 08198 SpinLockRelease(&xlogctl->info_lck);
>
> Note the use of capital XLogCtl->lastCheckPoint, which is not the
> volatile pointer.
That looks like a bug to me.
Come to think of it, the whole convention of using a lower-case
variant of the original pointer variable name seems like a foot-gun,
given the harmful and indeed very subtle consequences of making this
error.
I count 98 SpinLockAcquire() call sites (of which only a minority use
this convention, which is mostly within xlog.c, I think). Is it worth
instituting an alternative convention to make this kind of misuse more
obvious? This went unnoticed since February 2009.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services