Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2012-08-23 Thread Andres Freund
Hi,

While debugging an instance of this bug I noticed that plperlu always removes 
the SIGFPE handler and sets it to ignore:


andres@awork2:~$ psql -p 5435 -U postgres -h /var/run/postgresql test
Timing is on.
psql (9.1devel, server 9.1.5)
Type help for help.

test=# SELECT pg_backend_pid();
 pg_backend_pid 

   9287

root@awork2:/home/andres# grep -E '^Sig(Cgt|Ign)' /proc/9287/status|awk 
'{print $2}'
01301800
000180006287

test=# DO LANGUAGE plperlu ;

root@awork2:/home/andres# grep -E '^Sig(Cgt|Ign)' /proc/9287/status|awk 
'{print $2}'
01301880
000180006207

Note the 8'th bit being unset in SigCgt and set in SigIgn. Thats SIGFPE...

Not sure how relevant this really is, but it could cause errors to be 
ignored...

Greetings,

Andres
-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-10 Thread Andrew Dunstan



On 08/09/2011 04:32 PM, Tom Lane wrote:

Andrew Dunstanand...@dunslane.net  writes:

On 08/09/2011 12:22 PM, Tom Lane wrote:

No.  As I pointed out upthread, the instant somebody changes the SIGALRM
handler to a non-Postgres-aware one, you are already at risk of failure.
Setting it back later is just locking the barn door after the horses
left.  Institutionalizing such a non-fix globally is even worse.

So what's your suggestion? I know what you said you'd like, but it
doesn't appear at all practical to me.

[ shrug... ]  Installing a perl module that mucks with the signal
handlers is in the don't do that category.  A kluge such as you
suggest will not get it out of that category; all it will do is add
useless overhead for people who are following the rules.




Well, knowing what a given module might do isn't always easy (see 
below). I don't much like saying to people I told you so, especially 
when following the advice isn't necessarily straightforward.


After some experimentation, I found that, at least on my system, if LWP 
uses Crypt::SSLeay for https requests then it sets an alarm handler, but 
if instead it uses IO::Socket::SSL an alarm handler is not set. So the 
answer to the OP's original problem is probably make sure you have 
IO::Socket::SSL installed and that Crypt::SSLeay is not installed.



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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-10 Thread David E. Wheeler
On Aug 10, 2011, at 9:44 AM, Andrew Dunstan wrote:

 After some experimentation, I found that, at least on my system, if LWP uses 
 Crypt::SSLeay for https requests then it sets an alarm handler, but if 
 instead it uses IO::Socket::SSL an alarm handler is not set. So the answer to 
 the OP's original problem is probably make sure you have IO::Socket::SSL 
 installed and that Crypt::SSLeay is not installed.

I think I'd also complain via bug-crypt-ssl...@rt.cpan.org that a library ought 
not to set signal handlers.

Best,

David


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-10 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 08/09/2011 04:32 PM, Tom Lane wrote:
 [ shrug... ]  Installing a perl module that mucks with the signal
 handlers is in the don't do that category.  A kluge such as you
 suggest will not get it out of that category; all it will do is add
 useless overhead for people who are following the rules.

 Well, knowing what a given module might do isn't always easy (see 
 below). I don't much like saying to people I told you so, especially 
 when following the advice isn't necessarily straightforward.

I'm not thrilled with it either, but since we have no proposed patch
that would actually make it *safe* for perl modules to muck with the
signal handlers, I see no other alternative.  A patch that simply makes
it a shade less unsafe isn't really an improvement, especially when it
has other disadvantages.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-09 Thread Andrew Dunstan



On 08/08/2011 05:03 AM, Tim Bunce wrote:


After giving it some more thought it seems reasonable to simply force the
SIGALRM handler back to postgres when a plperlu function returns:

 pqsignal(SIGALRM, handle_sig_alarm);




Maybe we need to do this in some more centralized spot. It seems 
unlikely that this problem is unique to plperlu, or even just confined 
to PLs.


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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-09 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 08/08/2011 05:03 AM, Tim Bunce wrote:
 After giving it some more thought it seems reasonable to simply force the
 SIGALRM handler back to postgres when a plperlu function returns:

 pqsignal(SIGALRM, handle_sig_alarm);

 Maybe we need to do this in some more centralized spot. It seems 
 unlikely that this problem is unique to plperlu, or even just confined 
 to PLs.

No.  As I pointed out upthread, the instant somebody changes the SIGALRM
handler to a non-Postgres-aware one, you are already at risk of failure.
Setting it back later is just locking the barn door after the horses
left.  Institutionalizing such a non-fix globally is even worse.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-09 Thread Andrew Dunstan



On 08/09/2011 12:22 PM, Tom Lane wrote:

Andrew Dunstanand...@dunslane.net  writes:

On 08/08/2011 05:03 AM, Tim Bunce wrote:

After giving it some more thought it seems reasonable to simply force the
SIGALRM handler back to postgres when a plperlu function returns:
pqsignal(SIGALRM, handle_sig_alarm);

Maybe we need to do this in some more centralized spot. It seems
unlikely that this problem is unique to plperlu, or even just confined
to PLs.

No.  As I pointed out upthread, the instant somebody changes the SIGALRM
handler to a non-Postgres-aware one, you are already at risk of failure.
Setting it back later is just locking the barn door after the horses
left.  Institutionalizing such a non-fix globally is even worse.




So what's your suggestion? I know what you said you'd like, but it 
doesn't appear at all practical to me.


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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-09 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 08/09/2011 12:22 PM, Tom Lane wrote:
 No.  As I pointed out upthread, the instant somebody changes the SIGALRM
 handler to a non-Postgres-aware one, you are already at risk of failure.
 Setting it back later is just locking the barn door after the horses
 left.  Institutionalizing such a non-fix globally is even worse.

 So what's your suggestion? I know what you said you'd like, but it 
 doesn't appear at all practical to me.

[ shrug... ]  Installing a perl module that mucks with the signal
handlers is in the don't do that category.  A kluge such as you
suggest will not get it out of that category; all it will do is add
useless overhead for people who are following the rules.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-08 Thread Alex Hunsaker
On Sun, Aug 7, 2011 at 17:06, Tim Bunce tim.bu...@pobox.com wrote:

 On Sat, Aug 06, 2011 at 12:37:28PM -0600, Alex Hunsaker wrote:
 ...
 Find attached a version that does the equivalent of local %SIG for
 each pl/perl(u) call.

 +     gv = gv_fetchpv(SIG, 0, SVt_PVHV);
 +     save_hash(gv);                  /* local %SIG */

 ... [ local %SIG dosn't work ] The %SIG does become empty but the OS
 level handlers, even those installed by perl, *aren't changed*:

Looks like I trusted in $SIG{'ALRM'} being undef after it had been set
in a different scope too much :-( Thanks for pointing this out.

 That sure seems like a bug (I'll check with the perl5-porters list).

Well even if it was deemed a bug, it dont do us any good.

 Localizing an individual element of %SIG works fine.
 In C that's something like this (untested):

    hv = gv_fetchpv(SIG, 0, SVt_PVHV);
    keysv = ...SV containing ALRM...
    he = hv_fetch_ent(hv, keysv, 0, 0);
    if (he) {  /* arrange to restore existing elem */
        save_helem_flags(hv, keysv, HeVAL(he), SAVEf_SETMAGIC);
    }
    else {     /* arrange to delete a new elem */
        SAVEHDELETE(hv, keysv);
    }

I played with this a bit... and found yes, it locals them but no it
does not fix the reported problem. After playing with things a bit
more I found even local $SIG{'ALRM'} = .,..; alarm(1); still results
in postgres crashing. To wit, local does squat. AFAICT it just resets
the signal handler back to the default with SIG_DFL. (Which in
hindsight I don't know what else I expected it to-do...)

So I think for this to be robust we would have to detect what signals
they set and then reset those back to what postgres wants. Doable, but
is it worth it? Anyone else have any bright ideas?

Find below my test case and attached a patch that locals individual
%SIG elements the way mentioned above.

= set statement_timeout to '5s';
SET

= create or replace function test_alarm() returns void as $$ local
$SIG{'ALRM'} = sub { warn alarm; }; alarm(1); sleep 2; $$ language
plperlu;
CREATE FUNCTION

= select test_alarm();
WARNING:  alarm at line 1.
CONTEXT:  PL/Perl function test_alarm
 test_alarm


(1 row)

= select pg_sleep(6);
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: Failed.

Server Log:
WARNING:  alarm at line 1.
CONTEXT:  PL/Perl function test_alarm
LOG:  server process (PID 32659) was terminated by signal 14: Alarm clock
LOG:  terminating any other active server processes
WARNING:  terminating connection because of crash of another server process
DETAIL:  The postmaster has commanded this server process to roll back
the current transaction and exit, because another server process
exited abnormally and possibly corrupted shared memory.
HINT:  In a moment you should be able to reconnect to the database and
repeat your command.
FATAL:  the database system is in recovery mode
*** a/src/pl/plperl/expected/plperl.out
--- b/src/pl/plperl/expected/plperl.out
***
*** 639,641  CONTEXT:  PL/Perl anonymous code block
--- 639,643 
  DO $do$ use warnings FATAL = qw(void) ; my @y; my $x = sort @y; 1; $do$ LANGUAGE plperl;
  ERROR:  Useless use of sort in scalar context at line 1.
  CONTEXT:  PL/Perl anonymous code block
+ DO $do$ die SIG ALRM is set: $SIG{'ALRM'} if($SIG{'ALRM'}); $SIG{'ALRM'} = sub { print alarm!\n}; $do$ LANGUAGE plperl;
+ DO $do$ die SIG ALRM is set: $SIG{'ALRM'} if($SIG{'ALRM'}); $do$ LANGUAGE plperl;
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 268,273  static void plperl_inline_callback(void *arg);
--- 268,275 
  static char *strip_trailing_ws(const char *msg);
  static OP  *pp_require_safe(pTHX);
  static void activate_interpreter(plperl_interp_desc *interp_desc);
+ static void local_sigs(void);
+ static void local_sig(HV *hv, SV *tmpsv, const char *signame);
  
  #ifdef WIN32
  static char *setlocale_perl(int category, char *locale);
***
*** 1901,1906  plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
--- 1903,1910 
  	ENTER;
  	SAVETMPS;
  
+ 	local_sigs();
+ 
  	PUSHMARK(SP);
  	EXTEND(sp, desc-nargs);
  
***
*** 1968,1973  plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
--- 1972,2028 
  	return retval;
  }
  
+ /*
+  * local all of our sig handlers some modules like LWP like to set an alarm sig
+  * handler for things like network timeouts, this can cause bad stuff to happen
+  * (not to mention what happens if someone sets USR1)
+  *
+  * for now we just local() them all so they should get reset back to what
+  * postgres expects when their pl function is done
+  */
+ static void
+ local_sigs(void)
+ {
+ 	HV	*hv;
+ 	SV	*sv = newSV(9);
+ 	int i;
+ 
+ 	hv = get_hv(SIG, 0);
+ 	if (!hv)
+ 		elog(ERROR, couldn't fetch %%SIG);
+ 
+ 	/*
+ 	 * char 

Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-08 Thread Tim Bunce
On Mon, Aug 08, 2011 at 01:23:08AM -0600, Alex Hunsaker wrote:
 On Sun, Aug 7, 2011 at 17:06, Tim Bunce tim.bu...@pobox.com wrote:
 
  Localizing an individual element of %SIG works fine.
  In C that's something like this (untested):
 
     hv = gv_fetchpv(SIG, 0, SVt_PVHV);
     keysv = ...SV containing ALRM...
     he = hv_fetch_ent(hv, keysv, 0, 0);
     if (he) {  /* arrange to restore existing elem */
         save_helem_flags(hv, keysv, HeVAL(he), SAVEf_SETMAGIC);
     }
     else {     /* arrange to delete a new elem */
         SAVEHDELETE(hv, keysv);
     }
 
 I played with this a bit... and found yes, it locals them but no it
 does not fix the reported problem. After playing with things a bit
 more I found even local $SIG{'ALRM'} = .,..; alarm(1); still results
 in postgres crashing. To wit, local does squat. AFAICT it just resets
 the signal handler back to the default with SIG_DFL. (Which in
 hindsight I don't know what else I expected it to-do...)

Ah, yes. Hindsight is great. I should have spotted that. Sorry.

 So I think for this to be robust we would have to detect what signals
 they set and then reset those back to what postgres wants. Doable, but
 is it worth it? Anyone else have any bright ideas?

I'm only considering ALRM. At least that's the only one that seems worth
offering some limited support for. The others fall under don't do that.

After giving it some more thought it seems reasonable to simply force the
SIGALRM handler back to postgres when a plperlu function returns:

pqsignal(SIGALRM, handle_sig_alarm);

Tim.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-07 Thread Tim Bunce
[I've included a summary of the thread and Bcc'd this to perl5-porters
for a sanity check. Please trim heavily when replying.]

On Thu, Aug 04, 2011 at 09:42:31AM -0400, Andrew Dunstan wrote:
 
 So doesn't this look like a bug in the perl module that sets the
 signal handler and doesn't restore it?
 
 What happens if you wrap the calls to the module like this?:
 
 {
 local $SIG{ALRM};
 # do LWP stuff here
 }
 return 'OK';
 
 That should restore the old handler on exit from the block.
 
 I think if you use a perl module that monkeys with the signal
 handlers for any signal postgres uses all bets are off.

On Thu, Aug 04, 2011 at 10:28:45AM -0400, Tom Lane wrote:
 
  Sure, but how expensive would it be for pl/perl to do this
  automatically ?
 
 How can anything like that possibly work with any reliability
 whatsoever?  If the signal comes in, you don't know whether it was
 triggered by the event Postgres expected, or the event the perl module
 expected, and hence there's no way to deliver it to the right signal
 handler (not that the code you're describing is even trying to do that).
 
 What *I'd* like is a way to prevent libperl from touching the host
 application's signal handlers at all.  Sadly, Perl does not actually
 think of itself as an embedded library, and therefore thinks it owns all
 resources of the process and can diddle them without anybody's
 permission.

The PERL_IMPLICIT_SYS mechanism addresses this. Unfortunately it only
works with USE_ITHREADS on Windows currently.
http://perldoc.perl.org/perlguts.html#Future-Plans-and-PERL_IMPLICIT_SYS

On Thu, Aug 04, 2011 at 04:09:47PM -0600, Alex Hunsaker wrote:
 
 Well we can't prevent perl XS (aka C) from messing with signals (and
 other modules like POSIX that expose things like sigprocmask,
 siglongjump etc.) , but we could prevent plperl(u) from playing with
 signals on the perl level ala %SIG.
 
 [ IIRC I proposed doing something about this when we were talking
 about the whole Safe mess, but I think there was too much other
 discussion going on at the time :-) ]
 
 Mainly the options im thinking about are:
 1) if anyone touches %SIG die
 2) turn %SIG into a regular hash so people can set/play with %SIG, but
 it has no real effect.
 3) local %SIG before we call their trigger function. This lets signals
 still work while in trigger scope (like we do for %_TD)
 4) if we can't get any of the above to work we can save each %SIG
 handler before and restore them after each trigger call. (mod_perl
 does something similar so Im fairly certain we should be able to get
 that to work)

On Thu, Aug 4, 2011 at 16:34, David E. Wheeler da...@kineticode.com wrote:

 1) if anyone touches %SIG die
 2) turn %SIG into a regular hash so people can set/play with %SIG, but
 it has no real effect.

 These would disable stuff like $SIG{__WARN__} and $SIG{__DIE__}, which would 
 be an unfortunate side-effect.

On Thu, Aug 04, 2011 at 07:52:45PM -0400, Tom Lane wrote:
 
 The scenario I was imagining was:
 
 1. perl temporarily takes over SIGALRM.
 
 2. while perl function is running, statement_timeout expires, causing
 SIGALRM to be delivered.
 
 3. perl code is probably totally confused, and even if it isn't,
 statement_timeout will not be enforced since Postgres won't ever get the
 interrupt.
 
 Even if you don't think statement_timeout is a particularly critical
 piece of functionality, similar interference with the delivery of, say,
 SIGUSR1 would be catastrophic.
 
 How do you propose to prevent this sort of problem?

I don't think there's complete solution for that particular scenario.
[Though redirecting the perl alarm() opcode to code that would check the
argument against the remaining seconds before statement_timeout expires,
might get close.]


On Thu, Aug 04, 2011 at 06:44:18PM -0600, Alex Hunsaker wrote:
 
  How do you propose to prevent this sort of problem?
 
 Well, I think that makes it unworkable.
 
 So back to #1 or #2.
 
 For plperlu sounds like we are going to need to disallow setting _any_
 signals (minus __DIE__ and __WARN__). I should be able to make it so
 when you try it gives you a warning something along the lines of
 plperl can't set signal handlers, ignoring
 
 For plperl I think we should probably do the same. It seems like
 Andrew might disagree though? Anyone else want to chime in on if
 plperl lets you muck with signal handlers?
 
 Im not entirely sure how much of this is workable, I still need to go
 through perl's guts and see. At the very worst I think we can mark
 each signal handler that is exposed in %SIG readonly (which would mean
 we would  die instead of warning), but I think I can make the warning
 variant workable as well.
 
 I also have not dug deep enough to know how to handle __WARN__ and
 __DIE__ (and exactly what limitations allowing those will impose). I
 still have some work at $day_job before I can really dig into this.

On Thu, Aug 04, 2011 at 09:40:57PM -0400, Andrew Dunstan wrote:
 
 

Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-07 Thread Andrew Dunstan



On 08/07/2011 07:06 PM, Tim Bunce wrote:


After a little digging and some discussion on the #p5p channel [thanks
to ilmari++ leont++ and sorear++ for their help] it seems that local(%SIG)
doesn't do what you might expect. The %SIG does become empty but the OS
level handlers, even those installed by perl, *aren't changed*:

$ perl -wE '$SIG{INT} = sub { say Foo}; { local %SIG; kill INT, $$; };'
Foo
And, even worse, they're not reset at scope exit:

$ perl -wE '$SIG{INT} = sub { say Foo}; { local %SIG; $SIG{INT} = sub {say Bar }} 
kill INT, $$;'
Bar

That sure seems like a bug (I'll check with the perl5-porters list).


Yeah, that seems very bad. :-(


Localizing an individual element of %SIG works fine.
In C that's something like this (untested):

 hv = gv_fetchpv(SIG, 0, SVt_PVHV);
 keysv = ...SV containing ALRM...
 he = hv_fetch_ent(hv, keysv, 0, 0);
 if (he) {  /* arrange to restore existing elem */
 save_helem_flags(hv, keysv,HeVAL(he), SAVEf_SETMAGIC);
 }
 else { /* arrange to delete a new elem */
 SAVEHDELETE(hv, keysv);
 }




Hmm. I think we'll need to test how much it's going to cost to add that 
to every plperl (or maybe just every plperlu) function call for the six 
or so signals we use.


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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-06 Thread Alex Hunsaker
On Fri, Aug 5, 2011 at 08:53, Andrew Dunstan and...@dunslane.net wrote:


 On 08/04/2011 11:23 PM, Alex Hunsaker wrote:

 [ ... don't let people set signal handlers postgres sets ]

 This whole thing is a massive over-reaction to a problem we almost certainly
 know how to fix fairly simply and relatively painlessly, and attempts
 (unsuccessfully, at least insofar as comprehensiveness is concerned) to fix
 a problem nobody's actually reported having AFAIK.

*shrug* OK.

Find attached a version that does the equivalent of local %SIG for
each pl/perl(u) call. I was only able to test on 5.14.1, but I looked
at 5.8.9 to make sure it looks like it works.

Its a tad slower (something like 0.00037ms per call), but uhh thats
quite acceptable IMHO (best of 5 runs):

= create or replace function simple() returns void as $$ $$ language plperl;
CREATE FUNCTION

-- pre patch
= select count(simple()) from generate_series(1, 1000);
Time: 10219.149 ms

-- patched
= select count(simple()) from generate_series(1, 1000);
Time: 13924.025 ms

Thoughts?
*** a/src/pl/plperl/expected/plperl.out
--- b/src/pl/plperl/expected/plperl.out
***
*** 639,641  CONTEXT:  PL/Perl anonymous code block
--- 639,643 
  DO $do$ use warnings FATAL = qw(void) ; my @y; my $x = sort @y; 1; $do$ LANGUAGE plperl;
  ERROR:  Useless use of sort in scalar context at line 1.
  CONTEXT:  PL/Perl anonymous code block
+ DO $do$ die $SIG{'ALARM'} set! if($SIG{'ALARM'}); $SIG{'ALARM'} = sub { print alarm!\n}; $do$ LANGUAGE plperl;
+ DO $do$ die $SIG{'ALARM'} set! if($SIG{'ALARM'}); $do$ LANGUAGE plperl;
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 1895,1906  plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
--- 1895,1912 
  {
  	dSP;
  	SV		   *retval;
+ 	GV		   *gv;
  	int			i;
  	int			count;
  
  	ENTER;
  	SAVETMPS;
  
+ 	gv = gv_fetchpv(SIG, 0, SVt_PVHV);
+ 	if (!gv)
+ 		elog(ERROR, couldn't fetch %%SIG);
+ 	save_hash(gv);			/* local %SIG */
+ 
  	PUSHMARK(SP);
  	EXTEND(sp, desc-nargs);
  
***
*** 1976,1981  plperl_call_perl_trigger_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo,
--- 1982,1988 
  	dSP;
  	SV		   *retval,
  			   *TDsv;
+ 	GV		   *gv;
  	int			i,
  count;
  	Trigger*tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
***
*** 1986,1995  plperl_call_perl_trigger_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo,
  	TDsv = get_sv(_TD, 0);
  	if (!TDsv)
  		elog(ERROR, couldn't fetch $_TD);
- 
  	save_item(TDsv);			/* local $_TD */
  	sv_setsv(TDsv, td);
  
  	PUSHMARK(sp);
  	EXTEND(sp, tg_trigger-tgnargs);
  
--- 1993,2006 
  	TDsv = get_sv(_TD, 0);
  	if (!TDsv)
  		elog(ERROR, couldn't fetch $_TD);
  	save_item(TDsv);			/* local $_TD */
  	sv_setsv(TDsv, td);
  
+ 	gv = gv_fetchpv(SIG, 0, SVt_PVHV);
+ 	if (!gv)
+ 		elog(ERROR, couldn't fetch %%SIG);
+ 	save_hash(gv);			/* local %SIG */
+ 
  	PUSHMARK(sp);
  	EXTEND(sp, tg_trigger-tgnargs);
  
*** a/src/pl/plperl/sql/plperl.sql
--- b/src/pl/plperl/sql/plperl.sql
***
*** 415,417  DO $do$ use strict; my $name = foo; my $ref = $$name; $do$ LANGUAGE plperl;
--- 415,420 
  -- check that we can use warnings (in this case to turn a warn into an error)
  -- yields ERROR:  Useless use of sort in scalar context.
  DO $do$ use warnings FATAL = qw(void) ; my @y; my $x = sort @y; 1; $do$ LANGUAGE plperl;
+ 
+ DO $do$ die $SIG{'ALARM'} set! if($SIG{'ALARM'}); $SIG{'ALARM'} = sub { print alarm!\n}; $do$ LANGUAGE plperl;
+ DO $do$ die $SIG{'ALARM'} set! if($SIG{'ALARM'}); $do$ LANGUAGE plperl;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-05 Thread Andrew Dunstan



On 08/04/2011 11:23 PM, Alex Hunsaker wrote:

On Thu, Aug 4, 2011 at 19:40, Andrew Dunstanand...@dunslane.net  wrote:


Let's slow down a bit. Nobody that we know of has encountered the problem
Tom's referring to, over all the years plperlu has been available. The
changes you're proposing have the potential to downgrade the usefulness of
plperlu considerably without fixing anything that's known to be an actual
problem. Instead of fixing a problem caused by using LWP you could well make
LWP totally unusable from plperlu.

Well, im not sure about it making LWP totally unusable... You could
always use statement_timeout if you were worried about it blocking
^_^.




Making users set statement_timeout would be a degradation in utility. 
For one thing it means you'd never be able to get back and handle an 
unresponsiveness reply. And it would be extra work.


(I don't use LWP in any plperlu code AFAIR, but I do use other things 
that could well want to set signals. At the very least a change like 
this would mandate a LOT of extra testing by my clients.)




And it still won't do a thing about signal handlers installed by C code.

And plperlu would be the tip of the iceberg. What about all the other PLs,
not to mention non-PL loadable modules?

Maybe the answer is to re-issue the appropriate pqsignals() instead of
doing the perl variant?

For PL/Perl(u) we could still disallow any signals the postmaster
uses, from my quick look that would be: HUP, INT, TERM, QUIT, ALRM,
PIPE, USR1, USR2, FPE. All we would need to do is restore ALRM.

Or am I too paranoid about someone shooting themselves in the foot via
USR1? (BTW you can set signals in plperl, but you can't call alarm()
or kill())




This whole thing is a massive over-reaction to a problem we almost 
certainly know how to fix fairly simply and relatively painlessly, and 
attempts (unsuccessfully, at least insofar as comprehensiveness is 
concerned) to fix a problem nobody's actually reported having AFAIK.


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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Andrew Dunstan



On 08/04/2011 09:07 AM, Hannu Krosing wrote:

Hi

I have been helping some people to debug a SIGALARM related crash
induced by using pl/perlu http get functionality

I have been so far able to repeat the crash only on Debian 64 bit
computers. DB create script and instructions for reproducing the crash
attached

The crash is related to something leaving begind a bad SIGALARM handler,
as it can be (kind of) fixed by resetting sigalarm to nothing using perl
function


So doesn't this look like a bug in the perl module that sets the signal 
handler and doesn't restore it?


What happens if you wrap the calls to the module like this?:

{
local $SIG{ALRM};
# do LWP stuff here
}
return 'OK';


That should restore the old handler on exit from the block.

I think if you use a perl module that monkeys with the signal handlers 
for any signal postgres uses all bets are off.



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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Hannu Krosing
On Thu, 2011-08-04 at 15:07 +0200, Hannu Krosing wrote:
 Hi
 
 I have been helping some people to debug a SIGALARM related crash
 induced by using pl/perlu http get functionality
 
 I have been so far able to repeat the crash only on Debian 64 bit
 computers. DB create script and instructions for reproducing the crash
 attached

Resending - the previous one was in pre-edit stage with
instructions/comments in estonian :(

 The crash is related to something leaving begind a bad SIGALARM handler,
 as it can be (kind of) fixed by resetting sigalarm to nothing using perl
 function 
 
 REATE OR REPLACE FUNCTION reset_sigalarm() RETURNS VOID
 LANGUAGE plperlu
 AS $_X$
$SIG{ALRM} = 'IGNORE';
 $_X$;
 
 ( unfortunately this hoses deadlock detection and statement_timeout )
 
 Environment where this crash does happen:
 
 Debian GNU/Linux 6.0  - x86-64
 openssl 0.9.8o-4squeeze1
 postgresql-9.0  9.0.4-1~bpo60+1 
 postgresql-plperl-9.0   9.0.4-1~bpo60+1 
 libwww-perl 5.836-1  
 
 Postgresql is installed from backports
 
 It does not happen on 32 bit ubuntu
 
 

-- 
---
Hannu Krosing
PostgreSQL Infinite Scalability and Performance Consultant
PG Admin Book: http://www.2ndQuadrant.com/books/
/*
Minimal scripy for reproducing the error

needed languages

LANGUAGE plpgsql
LANGUAGE plperlu
Perl module: LWP::UserAgent
Perl v5.10.0
LWP::UserAgent v5.813

NB! crash happens only with HTTPS: protocol
*/

CREATE TABLE crashtest
(
  id integer,
  val varchar
);

INSERT INTO crashtest (id, val) VALUES (1, '');

CREATE OR REPLACE FUNCTION crashtest()
  RETURNS character varying AS
$BODY$
begin
perform crashtest_request();
update crashtest set val = 'XXX' where id=1;
perform pg_sleep(2);
RETURN 'OK';
END;
$BODY$
  LANGUAGE plpgsql VOLATILE SECURITY DEFINER;

CREATE OR REPLACE FUNCTION crashtest_request()
  RETURNS text AS
$BODY$
use strict;
use LWP::UserAgent;
my $ua = LWP::UserAgent-new;
my $req = HTTP::Request-new(GET = 'https://encrypted.google.com/'); 
# my $req = HTTP::Request-new(GET = 'https://www.paypal.com/ee/'); 
my $res = $ua-request($req);
return 'OK';
$BODY$
  LANGUAGE plperlu VOLATILE;

/*
-- now, run in at least psql 2 connectios simultaneously
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
select crashtest();
*/



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Hannu Krosing
On Thu, 2011-08-04 at 09:42 -0400, Andrew Dunstan wrote:
 
 On 08/04/2011 09:07 AM, Hannu Krosing wrote:
  Hi
 
  I have been helping some people to debug a SIGALARM related crash
  induced by using pl/perlu http get functionality
 
  I have been so far able to repeat the crash only on Debian 64 bit
  computers. DB create script and instructions for reproducing the crash
  attached
 
  The crash is related to something leaving begind a bad SIGALARM handler,
  as it can be (kind of) fixed by resetting sigalarm to nothing using perl
  function
 
 So doesn't this look like a bug in the perl module that sets the signal 
 handler and doesn't restore it?
 
 What happens if you wrap the calls to the module like this?:
 
  {
  local $SIG{ALRM};
  # do LWP stuff here
  }
  return 'OK';
 
 
 That should restore the old handler on exit from the block.
 
 I think if you use a perl module that monkeys with the signal handlers 
 for any signal postgres uses all bets are off.

Sure, but how expensive would it be for pl/perl to do this
automatically ?

 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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Alvaro Herrera
Excerpts from Hannu Krosing's message of jue ago 04 09:53:40 -0400 2011:
 On Thu, 2011-08-04 at 09:42 -0400, Andrew Dunstan wrote:
  
  On 08/04/2011 09:07 AM, Hannu Krosing wrote:

   I have been helping some people to debug a SIGALARM related crash
   induced by using pl/perlu http get functionality
  
   I have been so far able to repeat the crash only on Debian 64 bit
   computers. DB create script and instructions for reproducing the crash
   attached
  
   The crash is related to something leaving begind a bad SIGALARM handler,
   as it can be (kind of) fixed by resetting sigalarm to nothing using perl
   function
  
  So doesn't this look like a bug in the perl module that sets the signal 
  handler and doesn't restore it?

I vaguely remember looking in the guts of LWP::UserAgent a few years ago
and being rather annoyed at the way it dealt with sigalrm -- it
interfered with other uses we had for the signal.  I think we had to run
a patched version of that module or something, not sure.

  What happens if you wrap the calls to the module like this?:
  
   {
   local $SIG{ALRM};
   # do LWP stuff here
   }
   return 'OK';
  
  
  That should restore the old handler on exit from the block.
  
 
 Sure, but how expensive would it be for pl/perl to do this
 automatically ?

Probably too much, but then since this is an untrusted pl my guess is
that it's OK to request the user to do it only in functions that need
it.  I wonder if we could have a check on return from a function that
the sighandler is still what we had before the function was called, to
help discover this problem.

  I think if you use a perl module that monkeys with the signal handlers 
  for any signal postgres uses all bets are off.

Yeah.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Tom Lane
Hannu Krosing ha...@krosing.net writes:
 On Thu, 2011-08-04 at 09:42 -0400, Andrew Dunstan wrote:
 On 08/04/2011 09:07 AM, Hannu Krosing wrote:
 The crash is related to something leaving begind a bad SIGALARM handler,

 So doesn't this look like a bug in the perl module that sets the signal 
 handler and doesn't restore it?
 I think if you use a perl module that monkeys with the signal handlers 
 for any signal postgres uses all bets are off.

 Sure, but how expensive would it be for pl/perl to do this
 automatically ?

How can anything like that possibly work with any reliability
whatsoever?  If the signal comes in, you don't know whether it was
triggered by the event Postgres expected, or the event the perl module
expected, and hence there's no way to deliver it to the right signal
handler (not that the code you're describing is even trying to do that).

What *I'd* like is a way to prevent libperl from touching the host
application's signal handlers at all.  Sadly, Perl does not actually
think of itself as an embedded library, and therefore thinks it owns all
resources of the process and can diddle them without anybody's
permission.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Andrew Dunstan



On 08/04/2011 09:53 AM, Hannu Krosing wrote:



What happens if you wrap the calls to the module like this?:

  {
  local $SIG{ALRM};
  # do LWP stuff here
  }
  return 'OK';


That should restore the old handler on exit from the block.

I think if you use a perl module that monkeys with the signal handlers
for any signal postgres uses all bets are off.

Sure, but how expensive would it be for pl/perl to do this
automatically ?




Probably not very. It could possibly be added to 
plc_perlboot.pl::mkfuncsrc() after the prolog, or maybe before.


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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Alexey Klyukin

On Aug 4, 2011, at 5:25 PM, Alvaro Herrera wrote:

 Excerpts from Hannu Krosing's message of jue ago 04 09:53:40 -0400 2011:
 On Thu, 2011-08-04 at 09:42 -0400, Andrew Dunstan wrote:
 
 On 08/04/2011 09:07 AM, Hannu Krosing wrote:
 
 I have been helping some people to debug a SIGALARM related crash
 induced by using pl/perlu http get functionality
 
 I have been so far able to repeat the crash only on Debian 64 bit
 computers. DB create script and instructions for reproducing the crash
 attached
 
 The crash is related to something leaving begind a bad SIGALARM handler,
 as it can be (kind of) fixed by resetting sigalarm to nothing using perl
 function
 
 So doesn't this look like a bug in the perl module that sets the signal 
 handler and doesn't restore it?
 
 I vaguely remember looking in the guts of LWP::UserAgent a few years ago
 and being rather annoyed at the way it dealt with sigalrm -- it
 interfered with other uses we had for the signal.  I think we had to run
 a patched version of that module or something, not sure.
 
 What happens if you wrap the calls to the module like this?:
 
 {
 local $SIG{ALRM};
 # do LWP stuff here
 }
 return 'OK';
 
 
 That should restore the old handler on exit from the block.
 
 
 Sure, but how expensive would it be for pl/perl to do this
 automatically ?
 
 Probably too much, but then since this is an untrusted pl my guess is
 that it's OK to request the user to do it only in functions that need
 it.  I wonder if we could have a check on return from a function that
 the sighandler is still what we had before the function was called, to
 help discover this problem.

If we can do that, than why won't we move a step further and restore an old
signal handler on mismatch?

--
Command Prompt, Inc.  http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Andrew Dunstan



On 08/04/2011 10:28 AM, Tom Lane wrote:

How can anything like that possibly work with any reliability
whatsoever?  If the signal comes in, you don't know whether it was
triggered by the event Postgres expected, or the event the perl module
expected, and hence there's no way to deliver it to the right signal
handler (not that the code you're describing is even trying to do that).


True.


What *I'd* like is a way to prevent libperl from touching the host
application's signal handlers at all.  Sadly, Perl does not actually
think of itself as an embedded library, and therefore thinks it owns all
resources of the process and can diddle them without anybody's
permission.




I'm not sure how perl (or any loadable library) could restrict that in 
loaded C code, which many perl modules call directly or indirectly. It's 
as open as, say, a loadable C function is in Postgres ;-) You have a 
gun. It's loaded. If you point it at your foot and pull the trigger 
don't blame us. I think you just need to be very careful about what you 
do with plperlu. Don't be surprised if things break.


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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Hannu Krosing
On Thu, 2011-08-04 at 10:28 -0400, Tom Lane wrote:
 Hannu Krosing ha...@krosing.net writes:
  On Thu, 2011-08-04 at 09:42 -0400, Andrew Dunstan wrote:
  On 08/04/2011 09:07 AM, Hannu Krosing wrote:
  The crash is related to something leaving begind a bad SIGALARM handler,
 
  So doesn't this look like a bug in the perl module that sets the signal 
  handler and doesn't restore it?
  I think if you use a perl module that monkeys with the signal handlers 
  for any signal postgres uses all bets are off.
 
  Sure, but how expensive would it be for pl/perl to do this
  automatically ?
 
 How can anything like that possibly work with any reliability
 whatsoever?  If the signal comes in, you don't know whether it was
 triggered by the event Postgres expected, or the event the perl module
 expected, and hence there's no way to deliver it to the right signal
 handler (not that the code you're describing is even trying to do that).
 
 What *I'd* like is a way to prevent libperl from touching the host
 application's signal handlers at all.  Sadly, Perl does not actually
 think of itself as an embedded library, and therefore thinks it owns all
 resources of the process and can diddle them without anybody's
 permission.

It then seems that it is a goo idea to treat any fiddling with
postgreSQL signal handlers as an error, and rise an ERROR if any signal
handler has changed between calling the function and return, in a way
suggested by Alvaro

This at least forces the developer to pay attention and in case of
pl/perl function use something like the 

 {
 local $SIG{ALRM};
 # do LWP stuff here
 }
 return 'OK';

trick suggested by Andrew Dunstan

I know that this is not the real solution, bu at least it is easier to
debug than leaving a round signal handlers pointing to non-existent
code, which will trigger next time the deadlock checker tries to run.

-- 
---
Hannu Krosing
PostgreSQL Infinite Scalability and Performance Consultant
PG Admin Book: http://www.2ndQuadrant.com/books/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Alex Hunsaker
On Thu, Aug 4, 2011 at 09:11, Andrew Dunstan and...@dunslane.net wrote:

 What *I'd* like is a way to prevent libperl from touching the host
 application's signal handlers at all.  Sadly, Perl does not actually
 think of itself as an embedded library, and therefore thinks it owns all
 resources of the process and can diddle them without anybody's
 permission.



 I'm not sure how perl (or any loadable library) could restrict that in
 loaded C code, which many perl modules call directly or indirectly. It's as
 open as, say, a loadable C function is in Postgres ;-) You have a gun. It's
 loaded. If you point it at your foot and pull the trigger don't blame us. I
 think you just need to be very careful about what you do with plperlu. Don't
 be surprised if things break.

Well we can't prevent perl XS (aka C) from messing with signals (and
other modules like POSIX that expose things like sigprocmask,
siglongjump etc.) , but we could prevent plperl(u) from playing with
signals on the perl level ala %SIG.

[ IIRC I proposed doing something about this when we were talking
about the whole Safe mess, but I think there was too much other
discussion going on at the time :-) ]

Mainly the options im thinking about are:
1) if anyone touches %SIG die
2) turn %SIG into a regular hash so people can set/play with %SIG, but
it has no real effect.
3) local %SIG before we call their trigger function. This lets signals
still work while in trigger scope (like we do for %_TD)
4) if we can't get any of the above to work we can save each %SIG
handler before and restore them after each trigger call. (mod_perl
does something similar so Im fairly certain we should be able to get
that to work)

Thoughts?

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread David E. Wheeler
On Aug 4, 2011, at 3:09 PM, Alex Hunsaker wrote:

 Mainly the options im thinking about are:
 1) if anyone touches %SIG die
 2) turn %SIG into a regular hash so people can set/play with %SIG, but
 it has no real effect.

These would disable stuff like $SIG{__WARN__} and $SIG{__DIE__}, which would be 
an unfortunate side-effect.

 3) local %SIG before we call their trigger function. This lets signals
 still work while in trigger scope (like we do for %_TD)

+1

 4) if we can't get any of the above to work we can save each %SIG
 handler before and restore them after each trigger call. (mod_perl
 does something similar so Im fairly certain we should be able to get
 that to work)

+1

Best,

David


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Alex Hunsaker
On Thu, Aug 4, 2011 at 16:34, David E. Wheeler da...@kineticode.com wrote:
 On Aug 4, 2011, at 3:09 PM, Alex Hunsaker wrote:

 Mainly the options im thinking about are:
 1) if anyone touches %SIG die
 2) turn %SIG into a regular hash so people can set/play with %SIG, but
 it has no real effect.

 These would disable stuff like $SIG{__WARN__} and $SIG{__DIE__}, which would 
 be an unfortunate side-effect.

Yeah,  good point.

 3) local %SIG before we call their trigger function. This lets signals
 still work while in trigger scope (like we do for %_TD)

 +1

That seems to be what most people up-thread thought as well. I dont
see it being too expensive. Ill see if I can whip something up today.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Tom Lane
Alex Hunsaker bada...@gmail.com writes:
 On Thu, Aug 4, 2011 at 16:34, David E. Wheeler da...@kineticode.com wrote:
 On Aug 4, 2011, at 3:09 PM, Alex Hunsaker wrote:
 3) local %SIG before we call their trigger function. This lets signals
 still work while in trigger scope (like we do for %_TD)

 +1

 That seems to be what most people up-thread thought as well. I dont
 see it being too expensive. Ill see if I can whip something up today.

The scenario I was imagining was:

1. perl temporarily takes over SIGALRM.

2. while perl function is running, statement_timeout expires, causing
SIGALRM to be delivered.

3. perl code is probably totally confused, and even if it isn't,
statement_timeout will not be enforced since Postgres won't ever get the
interrupt.

Even if you don't think statement_timeout is a particularly critical
piece of functionality, similar interference with the delivery of, say,
SIGUSR1 would be catastrophic.

How do you propose to prevent this sort of problem?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Alex Hunsaker
On Thu, Aug 4, 2011 at 17:52, Tom Lane t...@sss.pgh.pa.us wrote:
 Alex Hunsaker bada...@gmail.com writes:
 On Thu, Aug 4, 2011 at 16:34, David E. Wheeler da...@kineticode.com wrote:
 On Aug 4, 2011, at 3:09 PM, Alex Hunsaker wrote:
 3) local %SIG before we call their trigger function. This lets signals
 still work while in trigger scope (like we do for %_TD)

 +1

 That seems to be what most people up-thread thought as well. I dont
 see it being too expensive. Ill see if I can whip something up today.

 The scenario I was imagining was:
 [ $SIG{ALRM} + statement timeout-- what happens?]
 
 Even if you don't think statement_timeout is a particularly critical
 piece of functionality, similar interference with the delivery of, say,
 SIGUSR1 would be catastrophic.

Yipes, I see your point.

 How do you propose to prevent this sort of problem?

Well, I think that makes it unworkable.

So back to #1 or #2.

For plperlu sounds like we are going to need to disallow setting _any_
signals (minus __DIE__ and __WARN__). I should be able to make it so
when you try it gives you a warning something along the lines of
plperl can't set signal handlers, ignoring

For plperl I think we should probably do the same. It seems like
Andrew might disagree though? Anyone else want to chime in on if
plperl lets you muck with signal handlers?

Im not entirely sure how much of this is workable, I still need to go
through perl's guts and see. At the very worst I think we can mark
each signal handler that is exposed in %SIG readonly (which would mean
we would  die instead of warning), but I think I can make the warning
variant workable as well.

I also have not dug deep enough to know how to handle __WARN__ and
__DIE__ (and exactly what limitations allowing those will impose). I
still have some work at $day_job before I can really dig into this.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Andrew Dunstan



On 08/04/2011 08:44 PM, Alex Hunsaker wrote:

On Thu, Aug 4, 2011 at 17:52, Tom Lanet...@sss.pgh.pa.us  wrote:

Alex Hunsakerbada...@gmail.com  writes:

On Thu, Aug 4, 2011 at 16:34, David E. Wheelerda...@kineticode.com  wrote:

On Aug 4, 2011, at 3:09 PM, Alex Hunsaker wrote:

3) local %SIG before we call their trigger function. This lets signals
still work while in trigger scope (like we do for %_TD)

+1

That seems to be what most people up-thread thought as well. I dont
see it being too expensive. Ill see if I can whip something up today.

The scenario I was imagining was:
[ $SIG{ALRM} + statement timeout-- what happens?]

Even if you don't think statement_timeout is a particularly critical
piece of functionality, similar interference with the delivery of, say,
SIGUSR1 would be catastrophic.

Yipes, I see your point.


How do you propose to prevent this sort of problem?

Well, I think that makes it unworkable.

So back to #1 or #2.

For plperlu sounds like we are going to need to disallow setting _any_
signals (minus __DIE__ and __WARN__). I should be able to make it so
when you try it gives you a warning something along the lines of
plperl can't set signal handlers, ignoring

For plperl I think we should probably do the same. It seems like
Andrew might disagree though? Anyone else want to chime in on if
plperl lets you muck with signal handlers?

Im not entirely sure how much of this is workable, I still need to go
through perl's guts and see. At the very worst I think we can mark
each signal handler that is exposed in %SIG readonly (which would mean
we would  die instead of warning), but I think I can make the warning
variant workable as well.

I also have not dug deep enough to know how to handle __WARN__ and
__DIE__ (and exactly what limitations allowing those will impose). I
still have some work at $day_job before I can really dig into this.


Let's slow down a bit. Nobody that we know of has encountered the 
problem Tom's referring to, over all the years plperlu has been 
available. The changes you're proposing have the potential to downgrade 
the usefulness of plperlu considerably without fixing anything that's 
known to be an actual problem. Instead of fixing a problem caused by 
using LWP you could well make LWP totally unusable from plperlu.


And it still won't do a thing about signal handlers installed by C code.

And plperlu would be the tip of the iceberg. What about all the other 
PLs, not to mention non-PL loadable modules?


But we *can* fix the original problem reported, namely failure to 
restore signal handlers on function exit, with very little downside 
(assuming it's shown to be fairly cheap).



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


Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https

2011-08-04 Thread Alex Hunsaker
On Thu, Aug 4, 2011 at 19:40, Andrew Dunstan and...@dunslane.net wrote:

 Let's slow down a bit. Nobody that we know of has encountered the problem
 Tom's referring to, over all the years plperlu has been available. The
 changes you're proposing have the potential to downgrade the usefulness of
 plperlu considerably without fixing anything that's known to be an actual
 problem. Instead of fixing a problem caused by using LWP you could well make
 LWP totally unusable from plperlu.

Well, im not sure about it making LWP totally unusable... You could
always use statement_timeout if you were worried about it blocking
^_^.

 And it still won't do a thing about signal handlers installed by C code.

 And plperlu would be the tip of the iceberg. What about all the other PLs,
 not to mention non-PL loadable modules?

Maybe the answer is to re-issue the appropriate pqsignals() instead of
doing the perl variant?

For PL/Perl(u) we could still disallow any signals the postmaster
uses, from my quick look that would be: HUP, INT, TERM, QUIT, ALRM,
PIPE, USR1, USR2, FPE. All we would need to do is restore ALRM.

Or am I too paranoid about someone shooting themselves in the foot via
USR1? (BTW you can set signals in plperl, but you can't call alarm()
or kill())

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers