Oops, looks like query cancel is busted

regression=# begin;
BEGIN
regression=# insert into rtest_t1 values(1,2);
INSERT 287918 1
regression=# select * from tenk1, tenk1 t2;
-- press ^C here
Cancel request sent
ERROR: Query was cancelled.
ERROR: Query was cancelled.
FATAL 2: elog: error during error recovery, giving up!
pqReadData() -- backend closed the channel unexpectedly.
This probably means the backend terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.

The problem is that END_CRIT_SECTION does (in effect)
if (QueryCancel)
CancelQuery();
which is OK in the main line of a transaction, but definitely NOT ok
when we are already recovering from an error. Like, say, the error
triggered by CancelQuery. And there are END_CRIT_SECTION calls needing
to be executed in that path --- the one in RecordTransactionAbort
being the hardest to avoid.

I think we should do two things here:

1. Alter END_CRIT_SECTION to only check for ProcDiePending. It should
not attempt to sprinkle the whole system with hidden QueryCancel checks,
which is what it's in effect doing now.

2. Modify CancelQuery to clear QueryCancel before elog'ing. Similarly,
END_CRIT_SECTION should clear ProcDiePending before elog'ing that case.

Although either one of these would be sufficient to suppress the
particular case I'm seeing, I think both are good practice. For
example, it is NOT a good idea to be checking for QueryCancel in the
END_CRIT_SECTION at the end of RecordTransactionCommit. At that point
we've already committed and raising an error is silly. So point #1
seems like a good idea. Point #2 seems like a good idea to avoid
recursive errors from explicitly written QueryCancel checks in the
post-elog path of control. I cannot find any such at the moment
other than the one in END_CRIT_SECTION, but one might get added someday.

regards, tom lane