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 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
Did this get through? Hadn't seen anyone comment on it, and I thought
it was pretty major :P
Christopher Kings-Lynne wrote:
Show quoted text
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/A7067CChris
---------------------------(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 while processing 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
Attachments:
xact.difftext/plain; charset=US-ASCII; name=xact.diffDownload
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.176
diff -2 -c -r1.176 xact.c
*** src/backend/access/transam/xact.c 1 Aug 2004 20:57:59 -0000 1.176
--- src/backend/access/transam/xact.c 3 Aug 2004 11:08:02 -0000
***************
*** 2523,2532 ****
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)
--- 2523,2529 ----
target = CurrentTransactionState;
+ while(PointerIsValid(target))
+ {
/* we don't cross savepoint level boundaries */
if (target->savepointLevel != s->savepointLevel)
***************
*** 2534,2539 ****
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
errmsg("no such savepoint")));
- }
if (!PointerIsValid(target))
ereport(ERROR,
--- 2531,2539 ----
(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
errmsg("no such savepoint")));
+ if (PointerIsValid(target->name) && strcmp(target->name, name) == 0)
+ break;
+ target = target->parent;
+ }
if (!PointerIsValid(target))
ereport(ERROR,
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")));