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

Tue, 03 Aug 2004 09:44:05 +0800
Christopher Kings-Lynne <chriskl@familyhealth.com.au>
How to crash postgres using savepoints
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

Tue, 03 Aug 2004 13:37:21 +0800
Christopher Kings-Lynne <chriskl@familyhealth.com.au>
Re: How to crash postgres using savepoints
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=#





Tue, 03 Aug 2004 02:50:13 -0700
Jeff Davis <jdavis-pgsql@empires.org>
Re: How to crash postgres using savepoints
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
Tue, 3 Aug 2004 20:41:27 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: How to crash postgres using savepoints
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")));



Tue, 03 Aug 2004 04:02:24 -0700
Jeff Davis <jdavis-pgsql@empires.org>
Re: How to crash postgres using savepoints
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

Tue, 3 Aug 2004 22:25:08 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: How to crash postgres using savepoints
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")));

Tue, 03 Aug 2004 12:03:06 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: How to crash postgres using savepoints 