Re: Restore CurrentUserId only if 'prevUser' is valid when abort transaction

On Tue, Oct 23, 2018 at 02:40:30PM +0900, Michael Paquier wrote:
> Actually, as StartSubTransaction also switches to TRANS_START for a
> savepoint, if there is an error until the state is switched to
> TRANS_INPROGRESS then the code would fail to switch back to
> CurrentUserId even if it is set, and it should be switched. So that
> solution is not correct either as AtSubStart_ResourceOwner() or such
> could fail on memory allocation. That's unlikely going to happen, but
> it could.

So, I have spent a couple of hours today looking a bit more at the
problem, and I have hacked the attached patch that I am pretty happy
with:
- Normal transactions can rely on TRANS_START to decide if the security
context can be reset or not.
- When defining a savepoint, the subtransaction state is initialized by
PushTransaction() before being switched to its sub-in-progress state
when the query which created the savepoint commits. In this case, we
should call GetUserIdAndSecContext() just before switching the
transaction context.

The patch includes a set tweaks I used to inject some errors in specific
code paths and trigger failures, checking if a security context which
has been set is correctly reset:
- /tmp/error_start for the end of StartTransaction
- /tmp/error_sub for the end of StartSubTransaction
- /tmp/error_push for the end of PushTransaction.

Like on HEAD, this patch still triggers the following WARNING if
injecting an error in PushTransaction as StartSubTransaction has not
switched the status of the transaction yet:
AbortSubTransaction while in DEFAULT state

Another WARNING which can be reached is the following if injecting an
error in StartSubTransaction:
AbortSubTransaction while in START state

Per the set of routines called when starting the subtransaction, I think
that we ought to do as main transactions and silence this warning
equally. I am attaching the patch for review by others. Please note
that this includes the error injections to ease tests.
--
Michael