Fix segfault with CopyOut.

When "COPY relname TO STDOUT" is executed in the extended query
protocol mode, pgpool segfaulted.

When read_kind_from_backend() reads a message from backend, it
extracts the corresponding entry from the pending message queue when
processing extended query protocol messages. However, if the head of
the message queue is an "execute" message, some of incoming message
types are exceptional because other than CommandComplete message
(which means the execute message finishes) may come from backend. This
includes DataRow, ErrorResponse, NoticeMessage. Unfortunately we
overlooked that 'H' (CopyOutResponse) is in the group too. Thus when
CopyOutResponse comes from backend, the execute pending message is
removed. If the next message from frontend is Sync (it's often
happens), read_kind_from_backend() sets session_context->query_context
to NULL, and calls pool_unset_query_in_progress(), which accesses
session_context->query_context and segfaults.

The fix is, to add CopyOutResponse to the exception list. Just in
case, we also add 'd' (CopyData) and 'c' (CopyDone) to the list. This
may not be actually necessary since CopyData and CopyDone are
processced in CopyDataRows() though.

Add regression test case to 126.copy_hang (master and v4.7) or
076.copy_hang (v4.6 or before).

Author: Tatsuo Ishii <ishii@postgresql.org>
Reported-by: https://github.com/tetesh
Reviewed-by: Bo Peng <pengbo@sraoss.co.jp>
Discussion: https://github.com/pgpool/pgpool2/issues/133
Backpatch-through: v4.2

diff --git a/src/protocol/pool_process_query.c b/src/protocol/pool_process_query.c
index 9a23f86f06ec7bbfe1c8a15b286bcbbe44219125..96334f8e0abe100b91a6205304618fff3338fbc4 100644 (file)
--- a/src/protocol/pool_process_query.c
+++ b/src/protocol/pool_process_query.c
@@ -3847,9 +3847,10 @@ read_kind_from_backend(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend,
        /*
         * If we are in in streaming replication mode and we doing an extended
         * query, check the kind we just read.  If it's one of 'D' (data row), 'E'
-        * (error), or 'N' (notice), and the head of the pending message queue was
-        * 'execute', the message must not be pulled out so that next Command
-        * Complete message from backend matches the execute message.
+        * (error), 'N' (notice), 'H' (CopyOutResponse), 'd' (CopyData) or 'c'
+        * (CopyDone) and the head of the pending message queue was 'execute', the
+        * message must not be pulled out so that next Command Complete message
+        * from backend matches the execute message.
         *
         * Also if it's 't' (parameter description) and the pulled message was
         * 'describe', the message must not be pulled out so that the row
@@ -3858,7 +3859,9 @@ read_kind_from_backend(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend,
        if (SL_MODE && pool_is_doing_extended_query_message() && msg)
        {
                if ((msg->type == POOL_EXECUTE &&
-                        (*decided_kind == 'D' || *decided_kind == 'E' || *decided_kind == 'N')) ||
+                        (*decided_kind == 'D' || *decided_kind == 'E' ||
+                         *decided_kind == 'N' || *decided_kind == 'H' ||
+                         *decided_kind == 'd' || *decided_kind == 'c')) ||
                        (msg->type == POOL_DESCRIBE && *decided_kind == 't'))
                {
                        ereport(DEBUG5,

diff --git a/src/test/regression/tests/126.copy_hang/copy-out-expected b/src/test/regression/tests/126.copy_hang/copy-out-expected
new file mode 100644 (file)
index 0000000..270ee35
--- /dev/null
+++ b/src/test/regression/tests/126.copy_hang/copy-out-expected
@@ -0,0 +1,37 @@
+FE=> Query (query="CREATE TEMP TABLE copy_in_test AS SELECT I FROM generate_series(1,10) AS i")
+<= BE CommandComplete(SELECT 10)
+<= BE ReadyForQuery(I)
+FE=> Query (query="SELECT * FROM copy_in_test")
+<= BE RowDescription
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE DataRow
+<= BE CommandComplete(SELECT 10)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="", query="COPY copy_in_test TO STDOUT")
+FE=> Bind(stmt="", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE CopyOutResponse
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyData
+<= BE CopyDone
+<= BE CommandComplete(COPY 10)
+<= BE ReadyForQuery(I)

diff --git a/src/test/regression/tests/126.copy_hang/pgproto-copy-out.data b/src/test/regression/tests/126.copy_hang/pgproto-copy-out.data
new file mode 100644 (file)
index 0000000..6270765
--- /dev/null
+++ b/src/test/regression/tests/126.copy_hang/pgproto-copy-out.data
@@ -0,0 +1,9 @@
+'Q'    "CREATE TEMP TABLE copy_in_test AS SELECT I FROM generate_series(1,10) AS i"
+'Y'
+'Q'    "SELECT * FROM copy_in_test"
+'Y'
+'P'    ""      "COPY copy_in_test TO STDOUT"   0
+'B'    ""      ""      0       0       0
+'E'    ""      0
+'S'
+'Y'

diff --git a/src/test/regression/tests/126.copy_hang/test.sh b/src/test/regression/tests/126.copy_hang/test.sh
index 9e0f4c0ce36ce719531bdf0e7cfba9819a3eb2b3..6ed70a72e907a7fdc71793dafcae2770dd5f19d5 100755 (executable)
--- a/src/test/regression/tests/126.copy_hang/test.sh
+++ b/src/test/regression/tests/126.copy_hang/test.sh
@@ -64,4 +64,20 @@ if [ ! $? -eq 0 ];then
     ./shutdownall
     exit 1
 fi
+
+#
+# Test case for COPY OUT in extended query protocol mode segfaults.
+# since this creates temp table, prevent load balance
+echo "backend_weight1 = 0" >> etc/pgpool.conf
+echo "backend_weight2 = 0" >> etc/pgpool.conf
+# reload pgpool.conf and wait until the effect is apparent
+./pgpool_reload
+sleep 1
+# run test script
+$PGPROTO -d test -f ../pgproto-copy-out.data > copy-out-result 2>&1
+cmp ../copy-out-expected copy-out-result
+if [ ! $? -eq 0 ];then
+    ./shutdownall
+    exit 1
+fi
 ./shutdownall