On 08/08/2025 11:49, Jelte Fennema-Nio wrote:
> On Fri, 8 Aug 2025 at 10:42, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> I'm not sure how to best fix that. If we can't process a Notify message
>> because of out of memory, what should we do?
>>
>> a) silently drop the Notify messsage.
>> b) report an error on the next query
>> c) drop the connection with the error.
>>
>> You went with c) in getBackendKeyData(), which makes sense since that
>> happens when establishing a connection. But Notify messages can be
>> received at any time. b) is appealing, but I'm a little worried if we
>> can manage the state correctly. Returning an error implies that the
>> transaction is aborted, but since this error is generated internally in
>> libpq, the transaction is not affected.
>>
>> I spotted one more case in pqSaveParameterStatus(): if the malloc()
>> there fails, it just silently skips storing the parameter, so that a
>> later call to PQparameterStatus() will not find it. That also seems bad.
>
> Nice finds. I think c) would be a very defensible position even for
> these two. Sure these two messages might not be critical to always
> receive correctly for all clients, but for some they might. And even
> for the ones where it's not critical, if malloc fails for these
> allocations, it's likely a next malloc for some other purpose will
> also fail. Relieving memory pressure by cleaning the connection seems
> a pretty sensible thing to do in such a case.
Makes sense.
Digging a little deeper still, there are many more cases. These
functions like getNotify(), getBackendKeyData(), getParameter() should
never return EOF. If they do, it's a sign of a protocol violation, and
we should drop the connection.
Those EOF returns are a remnants of handling protocol version 2.
Messages in protocol version 2 didn't have a length field, and we relied
on the functions to return EOF if there wasn't enough data in the
buffer, and come back later with more data. But nowadays, we always read
the whole message as indicated by the length field into memory before
calling them. So EOF means the message length didn't agree with the
fields within the message.
- Heikki
