Re: PL/Perl backed crashed during spi_exec_query

On Oct 31, 2009, at 7:30 PM, Tom Lane wrote:

> Alexey Klyukin <alexk@commandprompt.com> writes:
>> One of our customers is running 8.2.14 and use a couple of pl/perl  
>> and
>> pl/perlu functions written by CMD. Everything worked normally until
>> they tried to call one particular pl/perl function from pl/perl via
>> spi. It appears that a die call inside the callee just crashes the
>> backend.
>
> I think the critical point is actually that you're calling plperl from
> plperlu, and we're being careless about restoring the former  
> interpreter
> selection on error exit.  The attached patch moves the responsibility
> for that into plperl_call_handler, which already has a suitable
> PG_TRY block.

The patch solves the problem, thank you!

>
> 			regards, tom lane
>
> Index: plperl.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/pl/plperl/plperl.c,v
> retrieving revision 1.152
> diff -c -r1.152 plperl.c
> *** plperl.c	28 Sep 2009 17:31:12 -0000	1.152
> --- plperl.c	31 Oct 2009 17:27:14 -0000
> ***************
> *** 380,390 ****
>  	}
>  }
>
> !
>  static void
>  restore_context(bool old_context)
>  {
> ! 	if (trusted_context != old_context)
>  	{
>  		if (old_context)
>  			PERL_SET_CONTEXT(plperl_trusted_interp);
> --- 380,392 ----
>  	}
>  }
>
> ! /*
> !  * Restore previous interpreter selection, if two are active
> !  */
>  static void
>  restore_context(bool old_context)
>  {
> ! 	if (interp_state == INTERP_BOTH && trusted_context != old_context)
>  	{
>  		if (old_context)
>  			PERL_SET_CONTEXT(plperl_trusted_interp);
> ***************
> *** 870,878 ****
>  plperl_call_handler(PG_FUNCTION_ARGS)
>  {
>  	Datum		retval;
> ! 	plperl_call_data *save_call_data;
>
> - 	save_call_data = current_call_data;
>  	PG_TRY();
>  	{
>  		if (CALLED_AS_TRIGGER(fcinfo))
> --- 872,880 ----
>  plperl_call_handler(PG_FUNCTION_ARGS)
>  {
>  	Datum		retval;
> ! 	plperl_call_data *save_call_data = current_call_data;
> ! 	bool		oldcontext = trusted_context;
>
>  	PG_TRY();
>  	{
>  		if (CALLED_AS_TRIGGER(fcinfo))
> ***************
> *** 883,893 ****
> --- 885,897 ----
>  	PG_CATCH();
>  	{
>  		current_call_data = save_call_data;
> + 		restore_context(oldcontext);
>  		PG_RE_THROW();
>  	}
>  	PG_END_TRY();
>
>  	current_call_data = save_call_data;
> + 	restore_context(oldcontext);
>  	return retval;
>  }
>
> ***************
> *** 1226,1232 ****
>  	Datum		retval;
>  	ReturnSetInfo *rsi;
>  	SV		   *array_ret = NULL;
> - 	bool		oldcontext = trusted_context;
>  	ErrorContextCallback pl_error_context;
>
>  	/*
> --- 1230,1235 ----
> ***************
> *** 1376,1384 ****
>  	if (array_ret == NULL)
>  		SvREFCNT_dec(perlret);
>
> - 	current_call_data = NULL;
> - 	restore_context(oldcontext);
> -
>  	return retval;
>  }
>
> --- 1379,1384 ----
> ***************
> *** 1391,1397 ****
>  	Datum		retval;
>  	SV		   *svTD;
>  	HV		   *hvTD;
> - 	bool		oldcontext = trusted_context;
>  	ErrorContextCallback pl_error_context;
>
>  	/*
> --- 1391,1396 ----
> ***************
> *** 1491,1498 ****
>  	if (perlret)
>  		SvREFCNT_dec(perlret);
>
> - 	current_call_data = NULL;
> - 	restore_context(oldcontext);
>  	return retval;
>  }
>
> --- 1490,1495 ----