Обсуждение: Custom GUCs still a bit broken
It seems like Custom GUCs are still in need of some work, as shown in my
recent email. In particular, they are not transaction safe - if a
transaction attempts to do DefineCustomFooVariable() and that
transaction aborts, the placeholder setting that it used is already gone
by the time it tries to roll back GUC settings. I think this code at the
end of define_custom_variable()
/* * Free up as much as we conveniently can of the placeholder structure * (this neglects any
stackitems...) */ set_string_field(pHolder, pHolder->variable, NULL); set_string_field(pHolder,
&pHolder->reset_val,NULL);
free(pHolder);
needs to be removed and instead we need to save pHolder in a list along
with the GUC level, to be processed later by AtEOXact_GUC(), which would
do the right thing according to whether or not it had a commit or an abort.
I want to get this fixed before we consider custom settings for plperl
that have possible security implications.
Thoughts?
cheers
andrew
Where are we on this? --------------------------------------------------------------------------- Andrew Dunstan wrote: > > It seems like Custom GUCs are still in need of some work, as shown in my > recent email. In particular, they are not transaction safe - if a > transaction attempts to do DefineCustomFooVariable() and that > transaction aborts, the placeholder setting that it used is already gone > by the time it tries to roll back GUC settings. I think this code at the > end of define_custom_variable() > > /* > * Free up as much as we conveniently can of the placeholder > structure > * (this neglects any stack items...) > */ > set_string_field(pHolder, pHolder->variable, NULL); > set_string_field(pHolder, &pHolder->reset_val, NULL); > > free(pHolder); > > > needs to be removed and instead we need to save pHolder in a list along > with the GUC level, to be processed later by AtEOXact_GUC(), which would > do the right thing according to whether or not it had a commit or an abort. > > I want to get this fixed before we consider custom settings for plperl > that have possible security implications. > > Thoughts? > > cheers > > andrew > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
Nowhere, really. I tried to fix it, but could not come up with anything remotely clean. cheers andrew Bruce Momjian wrote: > Where are we on this? > > --------------------------------------------------------------------------- > > Andrew Dunstan wrote: > >> It seems like Custom GUCs are still in need of some work, as shown in my >> recent email. In particular, they are not transaction safe - if a >> transaction attempts to do DefineCustomFooVariable() and that >> transaction aborts, the placeholder setting that it used is already gone >> by the time it tries to roll back GUC settings. I think this code at the >> end of define_custom_variable() >> >> /* >> * Free up as much as we conveniently can of the placeholder >> structure >> * (this neglects any stack items...) >> */ >> set_string_field(pHolder, pHolder->variable, NULL); >> set_string_field(pHolder, &pHolder->reset_val, NULL); >> >> free(pHolder); >> >> >> needs to be removed and instead we need to save pHolder in a list along >> with the GUC level, to be processed later by AtEOXact_GUC(), which would >> do the right thing according to whether or not it had a commit or an abort. >> >> I want to get this fixed before we consider custom settings for plperl >> that have possible security implications. >> >> Thoughts? >> >> cheers >> >> andrew >> >> >> >> -- >> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgsql-hackers >> > >
Andrew Dunstan wrote: > > Nowhere, really. I tried to fix it, but could not come up with anything > remotely clean. So it is something for the TODO list or a 9.0 open item? --------------------------------------------------------------------------- > > cheers > > andrew > > > Bruce Momjian wrote: > > Where are we on this? > > > > --------------------------------------------------------------------------- > > > > Andrew Dunstan wrote: > > > >> It seems like Custom GUCs are still in need of some work, as shown in my > >> recent email. In particular, they are not transaction safe - if a > >> transaction attempts to do DefineCustomFooVariable() and that > >> transaction aborts, the placeholder setting that it used is already gone > >> by the time it tries to roll back GUC settings. I think this code at the > >> end of define_custom_variable() > >> > >> /* > >> * Free up as much as we conveniently can of the placeholder > >> structure > >> * (this neglects any stack items...) > >> */ > >> set_string_field(pHolder, pHolder->variable, NULL); > >> set_string_field(pHolder, &pHolder->reset_val, NULL); > >> > >> free(pHolder); > >> > >> > >> needs to be removed and instead we need to save pHolder in a list along > >> with the GUC level, to be processed later by AtEOXact_GUC(), which would > >> do the right thing according to whether or not it had a commit or an abort. > >> > >> I want to get this fixed before we consider custom settings for plperl > >> that have possible security implications. > >> > >> Thoughts? > >> > >> cheers > >> > >> andrew > >> > >> > >> > >> -- > >> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > >> To make changes to your subscription: > >> http://www.postgresql.org/mailpref/pgsql-hackers > >> > > > > -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
Bruce Momjian wrote: > Andrew Dunstan wrote: > >> Nowhere, really. I tried to fix it, but could not come up with anything >> remotely clean. >> > > So it is something for the TODO list or a 9.0 open item? > > It's not new, AFAIK. So arguably fixing it could just be a TODO. I don't have time right now to go down that rathole. cheers andrew
Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > Andrew Dunstan wrote: > > > >> Nowhere, really. I tried to fix it, but could not come up with anything > >> remotely clean. > >> > > > > So it is something for the TODO list or a 9.0 open item? > > > > > > It's not new, AFAIK. So arguably fixing it could just be a TODO. I don't > have time right now to go down that rathole. OK, added to TODO: Have custom GUCs be transaction safe http://archives.postgresql.org/message-by-id.php?4B577E9F.8000505@dunslane.net -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do