Lists: | pgsql-hackers |
---|
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Custom GUCs still a bit broken |
Date: | 2010-01-20 22:07:27 |
Message-ID: | 4B577E9F.8000505@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
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
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Custom GUCs still a bit broken |
Date: | 2010-03-03 23:23:06 |
Message-ID: | 201003032323.o23NN6020324@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Custom GUCs still a bit broken |
Date: | 2010-03-03 23:31:52 |
Message-ID: | 4B8EF168.5020006@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
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(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>
>
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Custom GUCs still a bit broken |
Date: | 2010-03-03 23:33:01 |
Message-ID: | 201003032333.o23NX1v22684@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | 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(at)postgresql(dot)org)
> >> To make changes to your subscription:
> >> http://www.postgresql.org/mailpref/pgsql-hackers
> >>
> >
> >
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Custom GUCs still a bit broken |
Date: | 2010-03-03 23:41:34 |
Message-ID: | 4B8EF3AE.5030701@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
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
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Custom GUCs still a bit broken |
Date: | 2010-03-03 23:52:23 |
Message-ID: | 201003032352.o23NqNF01655@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
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(dot)php?4B577E9F(dot)8000505(at)dunslane(dot)net
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do