Re: BackendKeyData is mandatory?

On 08/08/2025 17:01, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> I noticed that there's a similar, existing case in getNotify(), where
>> libpq just hangs if an allocation fails. To simulate that, apply this
>> change and use LISTEN/NOTIFY:
> 
>> --- a/src/interfaces/libpq/fe-protocol3.c
>> +++ b/src/interfaces/libpq/fe-protocol3.c
>> @@ -1587,7 +1587,7 @@ getNotify(PGconn *conn)
>>    	if (pqGets(&conn->workBuffer, conn))
>>    		return EOF;
>>    	/* must save name while getting extra string */
>> -	svname = strdup(conn->workBuffer.data);
>> +	svname = NULL;
>>    	if (!svname)
>>    		return EOF;
>>    	if (pqGets(&conn->workBuffer, conn))
> 
>> Returning EOF means "not enough data", which is wrong here just like in
>> getBackendKeyData().
> 
> The implication of "return EOF" is "try again later", which seems like
> about the best thing we can do.

We could:

a) report an error,
b) silently discard the async notification and move one, or
c) disconnect.

There's another malloc() call in the same getNotify() function, and if 
that fails, we silently discard the notification and move on (i.e. 
option b). So it's inconsistent.

The current behavior is "hang until more data is received from the 
server, then try again". I think any of the other options would be 
better. There's no guarantee that more data will ever arrive, the 
connection might be used just to wait for the notification.

> If memory serves, other places that construct query results do
> likewise.
It's a mix. Most functions that are related to queries, e.g. 
getRowDescriptions(), construct an error result object. If we run out of 
space in allocating the input buffer in pqCheckInBufferSpace(), we 
disconnect.

Another weird error handling: functions like pqGetInt() and pqGets() 
don't check that you don't read past the end of the message. They do 
check that they don't read past the input buffer, but the input buffer 
might contain another message after the current one, and the functions 
will merrily read past the message boundary. We do check for that later, 
report the "message contents do not agree with length ..." message, and 
resync. But it's weird that we don't check for that earlier already.

- Heikki