Thread: PG 8.1beta3 out soon
Core's current plan is to bundle 8.1beta3 tomorrow evening (Tuesday PM,
North American east coast time) for announcement Wednesday. Any last
minute bug fixes out there?
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > Core's current plan is to bundle 8.1beta3 tomorrow evening (Tuesday PM, > North American east coast time) for announcement Wednesday. Any last > minute bug fixes out there? Anyone able to duplicate my plperl bug? If it is genuine, I would really like to see it fixed for 8.1, as it's a showstopper. - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200510101855 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iD8DBQFDSvFvvJuQZxSWSsgRAuF/AJ9NFETn9KyTjjZh5qKLLyESMZkAgQCgpPHf lc4taOTI7maMmfTgkDjtIDs= =rLzu -----END PGP SIGNATURE-----
Greg Sabino Mullane wrote: >>Core's current plan is to bundle 8.1beta3 tomorrow evening (Tuesday PM, >>North American east coast time) for announcement Wednesday. Any last >>minute bug fixes out there? >> >> > >Anyone able to duplicate my plperl bug? If it is genuine, I would really >like to see it fixed for 8.1, as it's a showstopper. > > > > I take it you are referring to this: http://archives.postgresql.org/pgsql-bugs/2005-10/msg00095.php I don't think it's really a bug - it's a well known perl effect that has caught many people over the years, especially unwary users of Apache::Registry who fail to recognise that their scripts are being wrapped inside an anonymous subroutine. I took your example and simulated it entirely outside postgres/plperl to show that this is a pure perl effect. The script is like this: --------------------------------------------------- #!/usr/bin/perl use strict; use warnings; use constant { INFO => 'INFO' }; sub elog { print join(": ",@_),"\n"; } my $func = sub { my $_TD = $_[0]; shift; # use vars qw($_TD); local $_TD = shift; my $event = $_TD->{event}; elog(INFO, "Top event : $event"); my $newname = $_TD->{new}{a}; elog(INFO, "Top newname : $newname"); &subber($event); sub subber { no warnings qw(uninitialized); my $arg = shift; elog(INFO, join(" | ",caller(0))); elog(INFO, join(" | ",caller(1))); elog(INFO, "Sub global : $event"); elog(INFO, "Sub direct : $_TD->{event}"); my $newname = $_TD->{new}{a}; elog(INFO, "Sub newname : $newname"); } elog(INFO, "Bottom event : $event"); }; &$func({ new=>{a=>22},event=>'INSERT' }); &$func({ new=>{a=>33},event=>'UPDATE' }); &$func({ new=>{a=>44},event=>'INSERT' }); &$func({ new=>{a=>55},event=>'UPDATE' }); -------------------------------------------------- and the output is this - not the telltale first two lines: [andrew@alphonso ~]$ perl nonanontry.pl Variable "$event" will not stay shared at nonanontry.pl line 34. Variable "$_TD" will not stay shared at nonanontry.pl line 35. INFO: Top event : INSERT INFO: Top newname : 22 INFO: main | nonanontry.pl | 26 | main::subber | 1 | | | | 2 | UUUUUUUUUUUU INFO: main | nonanontry.pl | 43 | main::__ANON__ | 1 | | | | 2 | UUUUUUUUUUUU INFO: Sub global : INSERT INFO: Sub direct : INSERT INFO: Sub newname : 22 INFO: Bottom event : INSERT INFO: Top event : UPDATE INFO: Top newname : 33 INFO: main | nonanontry.pl | 26 | main::subber | 1 | | | | 2 | UUUUUUUUUUUU INFO: main | nonanontry.pl | 44 | main::__ANON__ | 1 | | | | 2 | UUUUUUUUUUUU INFO: Sub global : INSERT INFO: Sub direct : INSERT INFO: Sub newname : 22 INFO: Bottom event : UPDATE INFO: Top event : INSERT INFO: Top newname : 44 INFO: main | nonanontry.pl | 26 | main::subber | 1 | | | | 2 | UUUUUUUUUUUU INFO: main | nonanontry.pl | 45 | main::__ANON__ | 1 | | | | 2 | UUUUUUUUUUUU INFO: Sub global : INSERT INFO: Sub direct : INSERT INFO: Sub newname : 22 INFO: Bottom event : INSERT INFO: Top event : UPDATE INFO: Top newname : 55 INFO: main | nonanontry.pl | 26 | main::subber | 1 | | | | 2 | UUUUUUUUUUUU INFO: main | nonanontry.pl | 46 | main::__ANON__ | 1 | | | | 2 | UUUUUUUUUUUU INFO: Sub global : INSERT INFO: Sub direct : INSERT INFO: Sub newname : 22 INFO: Bottom event : UPDATE [andrew@alphonso ~]$ Now, if we swap the line that declares $_TD (which is just like it is in plperl.c) for the line underneath it, we could get rid of this effect (try it and see). I don't think it would have any memory leaks, but we'd need to check. However, that would only avoid the problem for what we know about, namely $_TD. The problem would remain for any lexical variable, because you are using a named nested subroutine which tries to access the lexical in its declaratory scope. Pass the hashref as an argument and have it only refer to the passed object rather than the one that is lexically visible and all should be well. My take: we should document this better, but it ain't broke so it don't need fixing, although I would not object strenuously to changing the behaviour of $_TD. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> My take: we should document this better, but it ain't broke so it don't
> need fixing,
Actually, my take on your analysis is that there should be a way to get
at "use warnings" (I assume that's disallowed in trusted plperl).
regards, tom lane
> Core's current plan is to bundle 8.1beta3 tomorrow evening (Tuesday PM, > North American east coast time) for announcement Wednesday. Any last > minute bug fixes out there? Not a bug fix, but this bug still hasn't been looked at: http://archives.postgresql.org/pgsql-hackers/2005-04/msg00499.php Chris
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>My take: we should document this better, but it ain't broke so it don't >>need fixing, >> >> > >Actually, my take on your analysis is that there should be a way to get >at "use warnings" (I assume that's disallowed in trusted plperl). > > > > Yes, we can't allow "use" in trusted code. But we could turn it on in plperl.c, just as we can turn on strict mode, and HEAD already has the infrastructure for logging lexical warnings - that's a new feature. I will investigate turning the switch. cheers andrew
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: > Not a bug fix, but this bug still hasn't been looked at: > http://archives.postgresql.org/pgsql-hackers/2005-04/msg00499.php I'm not really convinced that's a bug, and in any case it's not going to be dealt with in 8.1. regards, tom lane
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
NotDashEscaped: You need GnuPG to verify this message
> I don't think it's really a bug - it's a well known perl effect that has
> caught many people over the years, especially unwary users of
> Apache::Registry who fail to recognise that their scripts are being
> wrapped inside an anonymous subroutine.
Ah, ok, so it's a documentation bug! :)
> My take: we should document this better, but it ain't broke so it don't
> need fixing, although I would not object strenuously to changing the
> behaviour of $_TD.
Hmm...what if we did this?:
Index: plperl.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.92
diff -r1.92 plperl.c
671c671
< XPUSHs(sv_2mortal(newSVpv("my $_TD=$_[0]; shift;", 0)));
---
> XPUSHs(sv_2mortal(newSVpv("our $_TD=$_[0]; shift;", 0)));
--
Greg Sabino Mullane greg@turnstep.com
PGP Key: 0x14964AC8 200510110838
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----
iD8DBQFDS7J2vJuQZxSWSsgRAt7XAKC8D7HohA27CnaD7SVLrdKi80K49wCeI+o6
Tg9r3CSUeIV4872xuhZ0WBA=
=JRAX
-----END PGP SIGNATURE-----
Andrew Dunstan wrote:
> Tom Lane wrote:
>
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>
>>> My take: we should document this better, but it ain't broke so it
>>> don't need fixing,
>>>
>> Actually, my take on your analysis is that there should be a way to get
>> at "use warnings" (I assume that's disallowed in trusted plperl).
>
> Yes, we can't allow "use" in trusted code. But we could turn it on in
> plperl.c, just as we can turn on strict mode, and HEAD already has the
> infrastructure for logging lexical warnings - that's a new feature. I
> will investigate turning the switch.
>
I spoke a bit rashly here. The only way I have been able to make it work
so far in the Safe container is via the global -w flag - everything else
I tried failed, although it worked just fine for untrusted code. I don't
have lots of time to spend working out why. Another issue is that the
warnings pragma is fairly recent, and so might break on older perls
anyway, so just using -w might be the best way to go, if we do anything.
However, this turns on all warnings (e.g. use of uninitialized
variables) and the user can't turn them off. Still, that might not be a
bad thing. It will just cause the warnings to be logged, although
possibly a little verbosely.
That change at least is trivial.
So what's the consensus? "-w" or just document?
FYI, here is the perldiag man page extract covering the problem:
Variable "%s" will not stay shared
(W closure) An inner (nested) named subroutine is referencing a
lexical variable defined in an outer subroutine.
When the inner subroutine is called, it will probably see the value
of the outer subroutine’s variable as it was before and during the
*first* call to the outer subroutine; in this case, after the first
call to the outer subroutine is complete, the inner and outer sub-
routines will no longer share a common value for the variable. In
other words, the variable will no longer be shared.
Furthermore, if the outer subroutine is anonymous and references a
lexical variable outside itself, then the outer and inner subrou-
tines will never share the given variable.
This problem can usually be solved by making the inner subroutine
anonymous, using the "sub {}" syntax. When inner anonymous subs
that reference variables in outer subroutines are called or refer-
enced, they are automatically rebound to the current values of such
variables.
cheers
andrew
Greg Sabino Mullane wrote:
>
>Hmm...what if we did this?:
>
>Index: plperl.c
>===================================================================
>RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v
>retrieving revision 1.92
>diff -r1.92 plperl.c
>671c671
>< XPUSHs(sv_2mortal(newSVpv("my $_TD=$_[0]; shift;", 0)));
>---
>
>
>> XPUSHs(sv_2mortal(newSVpv("our $_TD=$_[0]; shift;", 0)));
>>
>>
>
>
>
>
That would probably work, but it would ONLY deal with the issue for
$_TD. In your function $event will still hit this problem.
see next email for sidcussion of warnings.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
>>> Actually, my take on your analysis is that there should be a way to get
>>> at "use warnings" (I assume that's disallowed in trusted plperl).
>>
>> Yes, we can't allow "use" in trusted code. But we could turn it on in
>> plperl.c, just as we can turn on strict mode, and HEAD already has the
>> infrastructure for logging lexical warnings - that's a new feature. I
>> will investigate turning the switch.
> I spoke a bit rashly here. The only way I have been able to make it work
> so far in the Safe container is via the global -w flag - everything else
> I tried failed, although it worked just fine for untrusted code. I don't
> have lots of time to spend working out why.
I think we'd best put this on the TODO list for 8.2. It's awfully late
in the cycle to be rushing through half-thought-out features ...
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > That would probably work, but it would ONLY deal with the issue for > $_TD. In your function $event will still hit this problem. Well, fixing $_TD would pretty much fix all the problems I've been having. As far as "$event", that is in my control and easily fixed by making it "our" as well. Some explicit warnings and best practices in the docs would be nice. Tom, can we possibly get that one-word patch into 8.1? It would go a long way towards making plperl more intuitive (and useful), and probably head off some future "bug" reports. Thanks, - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200510110941 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iD8DBQFDS8E4vJuQZxSWSsgRAvsYAJ9cz6yNdInOY2HbGJ5LKKBRFxe97QCgwoa4 r4sJrmQ4cHPE+NyYo+9dX1A= =Kfwb -----END PGP SIGNATURE-----
On Tue, Oct 11, 2005 at 08:51:01AM -0400, Andrew Dunstan wrote: > > > Andrew Dunstan wrote: > > >Tom Lane wrote: > > > >>Andrew Dunstan <andrew@dunslane.net> writes: > >> > >>>My take: we should document this better, but it ain't broke so it > >>>don't need fixing, > >>> > >>Actually, my take on your analysis is that there should be a way > >>to get at "use warnings" (I assume that's disallowed in trusted > >>plperl). > > > >Yes, we can't allow "use" in trusted code. But we could turn it on > >in plperl.c, just as we can turn on strict mode, and HEAD already > >has the infrastructure for logging lexical warnings - that's a new > >feature. I will investigate turning the switch. > > > > I spoke a bit rashly here. The only way I have been able to make it > work so far in the Safe container is via the global -w flag - > everything else I tried failed, although it worked just fine for > untrusted code. I don't have lots of time to spend working out why. > Another issue is that the warnings pragma is fairly recent, and so > might break on older perls anyway, so just using -w might be the > best way to go, if we do anything. However, this turns on all > warnings (e.g. use of uninitialized variables) and the user can't > turn them off. Still, that might not be a bad thing. It will just > cause the warnings to be logged, although possibly a little > verbosely. > > That change at least is trivial. > > So what's the consensus? "-w" or just document? +1 for -w. Documenting wouldn't hurt either. Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 510 893 6100 mobile: +1 415 235 3778 Remember to vote!
Greg Sabino Mullane wrote: >>That would probably work, but it would ONLY deal with the issue for >>$_TD. In your function $event will still hit this problem. >> >> > >Well, fixing $_TD would pretty much fix all the problems I've been having. >As far as "$event", that is in my control and easily fixed by making it >"our" as well. Some explicit warnings and best practices in the docs would >be nice. > >Tom, can we possibly get that one-word patch into 8.1? It would go a long >way towards making plperl more intuitive (and useful), and probably head >off some future "bug" reports. > > "our" only came in with Perl 5.6 ... I don't recall if we have declared support for earlier versions than that dead yet, although David Fetter and Joshua Drake have urged us to. I will add a note somewhere in the docs in the next few days advising against using named subroutines that refer to lexical variables from the enclosing scope. cheers andrew