aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2008-05-26 18:54:29 +0000
committerTom Lane <tgl@sss.pgh.pa.us>2008-05-26 18:54:29 +0000
commita3d9a2421ad3c16c22c82734a1d12be472bf9ba5 (patch)
tree509489c2514ee7d9dd448e3aa502c4dfccb7cb6f /src
parent8c2ac75c5cdb68aa240a8866dc828e14ee2b5e03 (diff)
downloadpostgresql-a3d9a2421ad3c16c22c82734a1d12be472bf9ba5.tar.gz
postgresql-a3d9a2421ad3c16c22c82734a1d12be472bf9ba5.zip
Fix an old corner-case bug in set_config_option: push_old_value has to be
called before, not after, calling the assign_hook if any. This is because push_old_value might fail (due to palloc out-of-memory), and in that case there would be no stack entry to tell transaction abort to undo the GUC assignment. Of course the actual assignment to the GUC variable hasn't happened yet --- but the assign_hook might have altered subsidiary state. Without a stack entry we won't call it again to make it undo such actions. So this is necessary to make the world safe for assign_hooks with side effects. Per a discussion a couple weeks ago with Magnus. Back-patch to 8.0. 7.x did not have the problem because it did not have allocatable stacks of GUC values.
Diffstat (limited to 'src')
-rw-r--r--src/backend/utils/misc/guc.c210
1 files changed, 99 insertions, 111 deletions
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f4466ddb141..bc38810a810 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
* Written by Peter Eisentraut <peter_e@gmx.net>.
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.454 2008/05/15 00:17:40 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.455 2008/05/26 18:54:29 tgl Exp $
*
*--------------------------------------------------------------------
*/
@@ -4630,6 +4630,10 @@ set_config_option(const char *name, const char *value,
source = conf->gen.reset_source;
}
+ /* Save old value to support transaction abort */
+ if (changeVal && !makeDefault)
+ push_old_value(&conf->gen, action);
+
if (conf->assign_hook)
if (!(*conf->assign_hook) (newval, changeVal, source))
{
@@ -4640,32 +4644,26 @@ set_config_option(const char *name, const char *value,
return false;
}
- if (changeVal || makeDefault)
+ if (changeVal)
+ {
+ *conf->variable = newval;
+ conf->gen.source = source;
+ }
+ if (makeDefault)
{
- /* Save old value to support transaction abort */
- if (!makeDefault)
- push_old_value(&conf->gen, action);
- if (changeVal)
+ GucStack *stack;
+
+ if (conf->gen.reset_source <= source)
{
- *conf->variable = newval;
- conf->gen.source = source;
+ conf->reset_val = newval;
+ conf->gen.reset_source = source;
}
- if (makeDefault)
+ for (stack = conf->gen.stack; stack; stack = stack->prev)
{
- GucStack *stack;
-
- if (conf->gen.reset_source <= source)
- {
- conf->reset_val = newval;
- conf->gen.reset_source = source;
- }
- for (stack = conf->gen.stack; stack; stack = stack->prev)
+ if (stack->source <= source)
{
- if (stack->source <= source)
- {
- stack->prior.boolval = newval;
- stack->source = source;
- }
+ stack->prior.boolval = newval;
+ stack->source = source;
}
}
}
@@ -4707,6 +4705,10 @@ set_config_option(const char *name, const char *value,
source = conf->gen.reset_source;
}
+ /* Save old value to support transaction abort */
+ if (changeVal && !makeDefault)
+ push_old_value(&conf->gen, action);
+
if (conf->assign_hook)
if (!(*conf->assign_hook) (newval, changeVal, source))
{
@@ -4717,32 +4719,26 @@ set_config_option(const char *name, const char *value,
return false;
}
- if (changeVal || makeDefault)
+ if (changeVal)
+ {
+ *conf->variable = newval;
+ conf->gen.source = source;
+ }
+ if (makeDefault)
{
- /* Save old value to support transaction abort */
- if (!makeDefault)
- push_old_value(&conf->gen, action);
- if (changeVal)
+ GucStack *stack;
+
+ if (conf->gen.reset_source <= source)
{
- *conf->variable = newval;
- conf->gen.source = source;
+ conf->reset_val = newval;
+ conf->gen.reset_source = source;
}
- if (makeDefault)
+ for (stack = conf->gen.stack; stack; stack = stack->prev)
{
- GucStack *stack;
-
- if (conf->gen.reset_source <= source)
- {
- conf->reset_val = newval;
- conf->gen.reset_source = source;
- }
- for (stack = conf->gen.stack; stack; stack = stack->prev)
+ if (stack->source <= source)
{
- if (stack->source <= source)
- {
- stack->prior.intval = newval;
- stack->source = source;
- }
+ stack->prior.intval = newval;
+ stack->source = source;
}
}
}
@@ -4781,6 +4777,10 @@ set_config_option(const char *name, const char *value,
source = conf->gen.reset_source;
}
+ /* Save old value to support transaction abort */
+ if (changeVal && !makeDefault)
+ push_old_value(&conf->gen, action);
+
if (conf->assign_hook)
if (!(*conf->assign_hook) (newval, changeVal, source))
{
@@ -4791,32 +4791,26 @@ set_config_option(const char *name, const char *value,
return false;
}
- if (changeVal || makeDefault)
+ if (changeVal)
+ {
+ *conf->variable = newval;
+ conf->gen.source = source;
+ }
+ if (makeDefault)
{
- /* Save old value to support transaction abort */
- if (!makeDefault)
- push_old_value(&conf->gen, action);
- if (changeVal)
+ GucStack *stack;
+
+ if (conf->gen.reset_source <= source)
{
- *conf->variable = newval;
- conf->gen.source = source;
+ conf->reset_val = newval;
+ conf->gen.reset_source = source;
}
- if (makeDefault)
+ for (stack = conf->gen.stack; stack; stack = stack->prev)
{
- GucStack *stack;
-
- if (conf->gen.reset_source <= source)
+ if (stack->source <= source)
{
- conf->reset_val = newval;
- conf->gen.reset_source = source;
- }
- for (stack = conf->gen.stack; stack; stack = stack->prev)
- {
- if (stack->source <= source)
- {
- stack->prior.realval = newval;
- stack->source = source;
- }
+ stack->prior.realval = newval;
+ stack->source = source;
}
}
}
@@ -4870,6 +4864,10 @@ set_config_option(const char *name, const char *value,
source = conf->gen.reset_source;
}
+ /* Save old value to support transaction abort */
+ if (changeVal && !makeDefault)
+ push_old_value(&conf->gen, action);
+
if (conf->assign_hook && newval)
{
const char *hookresult;
@@ -4907,40 +4905,32 @@ set_config_option(const char *name, const char *value,
}
}
- if (changeVal || makeDefault)
+ if (changeVal)
{
- /* Save old value to support transaction abort */
- if (!makeDefault)
- push_old_value(&conf->gen, action);
- if (changeVal)
+ set_string_field(conf, conf->variable, newval);
+ conf->gen.source = source;
+ }
+ if (makeDefault)
+ {
+ GucStack *stack;
+
+ if (conf->gen.reset_source <= source)
{
- set_string_field(conf, conf->variable, newval);
- conf->gen.source = source;
+ set_string_field(conf, &conf->reset_val, newval);
+ conf->gen.reset_source = source;
}
- if (makeDefault)
+ for (stack = conf->gen.stack; stack; stack = stack->prev)
{
- GucStack *stack;
-
- if (conf->gen.reset_source <= source)
- {
- set_string_field(conf, &conf->reset_val, newval);
- conf->gen.reset_source = source;
- }
- for (stack = conf->gen.stack; stack; stack = stack->prev)
+ if (stack->source <= source)
{
- if (stack->source <= source)
- {
- set_string_field(conf, &stack->prior.stringval,
- newval);
- stack->source = source;
- }
+ set_string_field(conf, &stack->prior.stringval,
+ newval);
+ stack->source = source;
}
- /* Perhaps we didn't install newval anywhere */
- if (newval && !string_field_used(conf, newval))
- free(newval);
}
}
- else if (newval)
+ /* Perhaps we didn't install newval anywhere */
+ if (newval && !string_field_used(conf, newval))
free(newval);
break;
}
@@ -4974,6 +4964,10 @@ set_config_option(const char *name, const char *value,
source = conf->gen.reset_source;
}
+ /* Save old value to support transaction abort */
+ if (changeVal && !makeDefault)
+ push_old_value(&conf->gen, action);
+
if (conf->assign_hook)
if (!(*conf->assign_hook) (newval, changeVal, source))
{
@@ -4985,32 +4979,26 @@ set_config_option(const char *name, const char *value,
return false;
}
- if (changeVal || makeDefault)
+ if (changeVal)
{
- /* Save old value to support transaction abort */
- if (!makeDefault)
- push_old_value(&conf->gen, action);
- if (changeVal)
+ *conf->variable = newval;
+ conf->gen.source = source;
+ }
+ if (makeDefault)
+ {
+ GucStack *stack;
+
+ if (conf->gen.reset_source <= source)
{
- *conf->variable = newval;
- conf->gen.source = source;
+ conf->reset_val = newval;
+ conf->gen.reset_source = source;
}
- if (makeDefault)
+ for (stack = conf->gen.stack; stack; stack = stack->prev)
{
- GucStack *stack;
-
- if (conf->gen.reset_source <= source)
- {
- conf->reset_val = newval;
- conf->gen.reset_source = source;
- }
- for (stack = conf->gen.stack; stack; stack = stack->prev)
+ if (stack->source <= source)
{
- if (stack->source <= source)
- {
- stack->prior.enumval = newval;
- stack->source = source;
- }
+ stack->prior.enumval = newval;
+ stack->source = source;
}
}
}