Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb
Date
Msg-id 1715953.1625592989@sss.pgh.pa.us
Whole thread Raw
In response to Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb  (Amul Sul <sulamul@gmail.com>)
Responses Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb  (Amul Sul <sulamul@gmail.com>)
List pgsql-hackers
Amul Sul <sulamul@gmail.com> writes:
> On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
>> I don't mind RelationGetSmgr(index)->smgr_rnode alone or
>> &variable->member alone and there's not the previous call to
>> RelationGetSmgr just above. How about using a temporary variable?
>> 
>> SMgrRelation srel = RelationGetSmgr(index);
>> smgrwrite(srel, ...);
>> log_newpage(srel->..);

> Understood.  Used a temporary variable for the place where
> RelationGetSmgr() calls are placed too close or in a loop.

[ squint... ]  Doesn't this risk introducing exactly the sort of
cache-clobber hazard we're trying to prevent?  That is, the above is
not safe unless you are *entirely* certain that there is not and never
will be any possibility of a relcache flush before you are done using
the temporary variable.  Otherwise it can become a dangling pointer.

The point of the static-inline function idea was to be cheap enough
that it isn't worth worrying about this sort of risky optimization.
Given that an smgr function is sure to involve some kernel calls,
I doubt it's worth sweating over an extra test-and-branch beforehand.
So where I was hoping to get to is that smgr objects are *only*
referenced by RelationGetSmgr() calls and nobody ever keeps any
other pointers to them across any non-smgr operations.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: visibility map corruption
Next
From: Alvaro Herrera
Date:
Subject: Re: Pipeline mode and PQpipelineSync()