Thread: How to crash postgres using savepoints
test=# begin; BEGIN test=# savepoint "A"; SAVEPOINT test=# rollback to a; server closed the connection unexpectedly This probably means the server terminated abnormally before or whileprocessing the request. LOG: server process (PID 45905) was terminated by signal 11 LOG: terminating any other active server processes The connection to the server was lost. Attempting reset: LOG: background writer process (PID 45899) exited with exit code 1 LOG: all server processes terminated; reinitializing LOG: database system was interrupted at 2004-08-03 09:42:11 WST LOG: checkpoint record is at 0/A7067C Chris
Did this get through? Hadn't seen anyone comment on it, and I thought it was pretty major :P Christopher Kings-Lynne wrote: > test=# begin; > BEGIN > test=# savepoint "A"; > SAVEPOINT > test=# rollback to a; > server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. > LOG: server process (PID 45905) was terminated by signal 11 > LOG: terminating any other active server processes > The connection to the server was lost. Attempting reset: LOG: background > writer process (PID 45899) exited with exit code 1 > LOG: all server processes terminated; reinitializing > LOG: database system was interrupted at 2004-08-03 09:42:11 WST > LOG: checkpoint record is at 0/A7067C > > Chris > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
On Mon, 2004-08-02 at 22:37, Christopher Kings-Lynne wrote: > Did this get through? Hadn't seen anyone comment on it, and I thought > it was pretty major :P > I'd just like to second your claims. I have a snapshot from 2004-08-02 and I appended a sequence of SQL commands that causes a crash for me. Regards, Jeff Davis Sequence that causes crash: test=# begin; BEGIN test=# savepoint a; SAVEPOINT test=# rollback to b; server closed the connection unexpectedly This probably means the server terminated abnormally before or whileprocessing the request. The connection to the server was lost. Attempting reset: LOG: server process (PID 23751) was terminated by signal 11 LOG: terminating any other active server processes LOG: background writer process (PID 20334) exited with exit code 1 LOG: all server processes terminated; reinitializing LOG: database system was interrupted at 2004-08-02 21:40:58 PDT LOG: checkpoint record is at 0/A70914 LOG: redo record is at 0/A70914; undo record is at 0/0; shutdown TRUE LOG: next transaction ID: 529; next OID: 25420 LOG: database system was not properly shut down; automatic recovery in progressLOG: record with zero length at 0/A70954 LOG: redo is not required LOG: database system is ready Succeeded. test=#
Attached is a patch fixing this. One question I do have: if (target->savepointLevel != s->savepointLevel) Will this ever be true in the current code? I cannot see anything setting savepointLevel explicitly. Gavin
On Tue, 2004-08-03 at 03:41, Gavin Sherry wrote:
> Attached is a patch fixing this.
>
> One question I do have:
>
> if (target->savepointLevel != s->savepointLevel)
>
> Will this ever be true in the current code? I cannot see anything setting
> savepointLevel explicitly.
>From reading the lists, it seems like that's allowing for some
functionality that was talked about but wasn't part of the semantics
agreed upon by the end of the discussion.
I have a question for you also. I just posted a patch at about the same
time you did (I sent it to pgsql-patches, but I haven't seen it appear
yet). Mine was a one-liner (appended to end of this email) and all it
did was add a check into the aforementioned line for a non-null target
pointer. My patch seemed to work, so I'd like to know why you changed
the structure around more. I did notice some things were a little
cleaner, so was it just clean-up or does my patch fail in some way?
Regards,Jeff
--- xact.c.old 2004-08-03 03:18:12.000000000 -0700
+++ xact.c 2004-08-03 03:19:05.000000000 -0700
@@ -2529,7 +2529,7 @@ target = target->parent;
/* we don't cross savepoint level boundaries */
- if (target->savepointLevel != s->savepointLevel)
+ if (PointerIsValid(target) && (target->savepointLevel !=
s->savepointLevel)) ereport(ERROR,
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION), errmsg("no such savepoint")));
On Tue, 3 Aug 2004, Jeff Davis wrote: > On Tue, 2004-08-03 at 03:41, Gavin Sherry wrote: > > Attached is a patch fixing this. > > > > One question I do have: > > > > if (target->savepointLevel != s->savepointLevel) > > > > Will this ever be true in the current code? I cannot see anything setting > > savepointLevel explicitly. > > >From reading the lists, it seems like that's allowing for some > functionality that was talked about but wasn't part of the semantics > agreed upon by the end of the discussion. Yes. Savepoint levels are covered in the spec I was more or less just wondering if there was any more code to come or if I'd missed something. > > I have a question for you also. I just posted a patch at about the same > time you did (I sent it to pgsql-patches, but I haven't seen it appear > yet). Mine was a one-liner (appended to end of this email) and all it > did was add a check into the aforementioned line for a non-null target > pointer. My patch seemed to work, so I'd like to know why you changed > the structure around more. I did notice some things were a little > cleaner, so was it just clean-up or does my patch fail in some way? Just a clean up. Gavin
Gavin Sherry <swm@linuxworld.com.au> writes:
> On Tue, 3 Aug 2004, Jeff Davis wrote:
>> I have a question for you also. I just posted a patch at about the same
>> time you did (I sent it to pgsql-patches, but I haven't seen it appear
>> yet). Mine was a one-liner (appended to end of this email) and all it
>> did was add a check into the aforementioned line for a non-null target
>> pointer. My patch seemed to work, so I'd like to know why you changed
>> the structure around more. I did notice some things were a little
>> cleaner, so was it just clean-up or does my patch fail in some way?
> Just a clean up.
Actually, there's an easier fix than either of these, which is just to
pull the savepointLevel test out of the loop and only do it after we've
found a candidate savepoint. There's no point in checking the level at
every loop iteration (especially since the feature isn't even being used
yet, as noted).
Attached is the patch I just applied (which also includes my own notions
about how to make the loop prettier...)
regards, tom lane
*** src/backend/access/transam/xact.c.orig Sun Aug 1 16:57:59 2004
--- src/backend/access/transam/xact.c Tue Aug 3 11:53:41 2004
***************
*** 2520,2541 **** Assert(PointerIsValid(name));
! target = CurrentTransactionState;
!
! while (target != NULL) { if (PointerIsValid(target->name) && strcmp(target->name, name) == 0)
break;
- target = target->parent;
-
- /* we don't cross savepoint level boundaries */
- if (target->savepointLevel != s->savepointLevel)
- ereport(ERROR,
- (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
- errmsg("no such savepoint"))); } if (!PointerIsValid(target)) ereport(ERROR,
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION), errmsg("no such savepoint")));
--- 2520,2538 ---- Assert(PointerIsValid(name));
! for (target = s; PointerIsValid(target); target = target->parent) { if (PointerIsValid(target->name)
&&strcmp(target->name, name) == 0) break; } if (!PointerIsValid(target))
+ ereport(ERROR,
+ (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
+ errmsg("no such savepoint")));
+
+ /* disallow crossing savepoint level boundaries */
+ if (target->savepointLevel != s->savepointLevel) ereport(ERROR,
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION), errmsg("no such savepoint")));