Thread: [Patch] PQconnectPoll() is blocked if target_session_attrs isread-write

Hi

# I rewrote my previous mail.

PQconnectPoll() is used as method for asynchronous using externally or internally.
If a caller confirms a socket ready for writing or reading that is
requested by return value of previous PQconnectPoll(), next PQconnectPoll()
must not be blocked. But if the caller specifies target_session_attrs to
'read-write', PQconnectPoll() may be blocked.

Detail:
If target_session_attrs is set to read-write, PQconnectPoll() calls
PQsendQuery("SHOW transaction_read_only") althogh previous return value was
PGRES_POLLING_READING not WRITING.
In result, PQsendQuery() may be blocked in pqsecure_raw_write().

I attach a patch.

Regards
Ryo Matsumura

Hello.

At Mon, 22 Jul 2019 02:28:22 +0000, "Matsumura, Ryo" <matsumura.ryo@jp.fujitsu.com> wrote in
<03040DFF97E6E54E88D3BFEE5F5480F74AC15BBD@G01JPEXMBYT04>
> Hi
> 
> # I rewrote my previous mail.
> 
> PQconnectPoll() is used as method for asynchronous using externally or internally.
> If a caller confirms a socket ready for writing or reading that is
> requested by return value of previous PQconnectPoll(), next PQconnectPoll()
> must not be blocked. But if the caller specifies target_session_attrs to
> 'read-write', PQconnectPoll() may be blocked.
> 
> Detail:
> If target_session_attrs is set to read-write, PQconnectPoll() calls
> PQsendQuery("SHOW transaction_read_only") althogh previous return value was
> PGRES_POLLING_READING not WRITING.
> In result, PQsendQuery() may be blocked in pqsecure_raw_write().
> 
> I attach a patch.
> 
> Regards
> Ryo Matsumura

First, this patch looks broken.

patched>         if (conn->sversion >= 70400 &&
patched>           conn->target_session_attrs != NULL &&
patched>           strcmp(conn->target_session_attrs, "read-write") == 0)
patched>         {
patched>         }

Perhaps you did cut-n-paste instead of copy-n-paste.

I'm not sure such a small write just after reading can block, but
doing that makes things tidy.

You also need to update the corresponding documentation.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Kyotaro-san

Thank you for your review.

> First, this patch looks broken.

I took a serious mistake.

> You also need to update the corresponding documentation.

I attach a new patch that includes updating of document.

Regards
Ryo Matasumura

From: Matsumura, Ryo [mailto:matsumura.ryo@jp.fujitsu.com]
> Detail:
> If target_session_attrs is set to read-write, PQconnectPoll() calls
> PQsendQuery("SHOW transaction_read_only") althogh previous return value
> was PGRES_POLLING_READING not WRITING.

The current code probably assumes that PQsendQuery() to send "SHOW transaction_read_only" shouldn't block, because the
messageis small enough to fit in the socket send buffer.  Likewise, the code in CONNECTION_AWAITING_RESPONSE case sends
authenticationdata using pg_fe_sendauth() without checking for the write-ready status.  OTOH, the code in
CONNECTION_MADEcase waits for write-ready status in advance before sending the startup packet.  That's because the
startuppacket could get large enough to cause pqPacketSend() to block.
 

So, I don't think the fix is necessary.

> In result, PQsendQuery() may be blocked in pqsecure_raw_write().

FWIW, if PQsendQuery() blocked during connection establishment, I think it should block in poll() called from .... from
pqWait(),because the libpq's socket is set non-blocking.
 


Regards
Takayuki Tsunakawa

Tsunakawa-san

Thank you for your comment.
I understand the sense. I don't require an explicit rule.

Regards
Ryo Matsumura