Message-ID: <f6fa92fe-8408-465d-b775-1e0f10cd499a@vondra.me>
Date: Thu, 11 Jun 2026 16:40:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Autoprewarm workers terminated due to a segmentation fault
To: Matheus Alcantara <matheusssilv97@gmail.com>,
 Glauber Batista <glauberrbatista@gmail.com>,
 pgsql-bugs@lists.postgresql.org, Melanie Plageman <melanieplageman@gmail.com>
References: 
 <CAO+_mTQgQyTYwDh=U8iTnsDmOGyWsZJjUV31SmEYwmw6_xY6Bw@mail.gmail.com>
 <DJ4TOYZGE7M8.1U3BNIRMQVXQX@gmail.com>
 <f4c7fe76-6b98-4d92-a090-2ba8184b8997@vondra.me>
 <DJ4VE1MCA80G.1AG5OOFGIWCI0@gmail.com>
Content-Language: en-US
From: Tomas Vondra <tomas@vondra.me>
In-Reply-To: <DJ4VE1MCA80G.1AG5OOFGIWCI0@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/f6fa92fe-8408-465d-b775-1e0f10cd499a%40vondra.me>
Precedence: bulk

On 6/10/26 00:25, Matheus Alcantara wrote:
> On Tue Jun 9, 2026 at 6:44 PM -03, Tomas Vondra wrote:
>> So how does it get advanced past the prewarm_stop_idx? I've been unable
>> to reproduce it locally, maybe it's platform-specific. The original
>> report was from ARM, are you on ARM too, Matheus?
>>
> 
> Yes, I'm also on ARM. I also set pg_prewarm.autoprewarm_interval=10s on
> postgresql.conf, not sure if it make more easier to reproduce.
> 
>> But AFAIK the code may not account for read stream callback updating the
>> pos to prewarm_stop_idx? The callback may end with (p->pos =
>> apw_state->prewarm_stop_idx), and that seems to be past the end of the
>> array.
>>
> 
> Yes, this is my understanding.
> 
>> That'd mean the proposed check is generally the correct way to fix this.
>> TBH it's not clear to me why this needs to set the *next* entry at the
>> end of the loop. Well, it does that so that the loop condition can use
>> 'blk', but that seems a bit fragile / confusing, and no one noticed the
>> issue.
>>
>> Maybe this would be a better way to write the while loop?
>>
>>     while (i < apw_state->prewarm_stop_idx)
>>     {
>>         blk = block_info[i];
>>
>>         if (blk.tablespace != tablespace ||
>>             blk.filenumber != filenumber)
>>             break;
>>
>>         ...
>>     }
>>
>>
> 
> Is attached patch what are you sugesting? If yes, I agree that looks
> better, it's more safe and easier to understand.
> 

Yeah, that's roughly what I had in mind.

After looking a bit closer, I think this bug was introduced by

    commit 6acab8bdbcda735ef47b1bb0ba2284d6c465cd88
    Author: Melanie Plageman <melanieplageman@gmail.com>
    Date:   Fri Apr 4 15:25:27 2025 -0400

    Refactor autoprewarm_database_main() in preparation for read stream

which happens to advance to the next block in a couple places

    blk = block_info[++i];

before we know it the incremented "i" is a valid element. The following
commit (d9c7911e1a5f adding the read stream) ends up doing the same
thing, except the index is incremented in a callback.

Melanie, do you agree with the proposed fix?


regards

-- 
Tomas Vondra