Re: PL/Perl backed crashed during spi_exec_query

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

> Alexey Klyukin <alexk(at)commandprompt(dot)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 ----