Re: pl/perl extension fails on Windows

Hi,

On Fri, Jul 28, 2017 at 4:20 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
> > On 07/27/2017 04:33 PM, Tom Lane wrote:
> >> So I was trying to figure a way to not include XSUB.h except in a very
> >> limited part of plperl, like ideally just the .xs files. It's looking
> >> like that would take nontrivial refactoring though :-(. Another problem
> >> is that this critical bit of the library API is in XSUB.h:
>
> > That's the sort of thing that prompted me to ask what was the minimal
> > set of defines required to fix the original problem (assuming such a
> > thing exists)
> > We haven't used PERL_IMPLICIT_CONTEXT to date, and without ill effect.
> > For example. it's in the ExtUtils::Embed::ccopts for the perl that
> > jacana and bowerbird happily build and test against.
>
> Well, actually, PERL_IMPLICIT_CONTEXT is turned on automatically in any
> MULTIPLICITY build, and since it changes all the Perl ABIs, we *are*
> relying on it. However, after further study of the Perl docs I noticed
> that we could dispense with XSUB.h if we defined PERL_NO_GET_CONTEXT
> (which turns the quoted stanza into a no-op). That results in needing to
> sprinkle plperl.c with "dTHX" declarations, as explained in perlguts.pod.
> They're slightly tricky to place correctly, because they load up a pointer
> to the current Perl interpreter, so you have to be wary of where to put
> them in functions that change interpreters. But I seem to have it working
> in the attached patch. (One benefit of doing this extra work is that it
> should be a bit more efficient, in that we load up a Perl interpreter
> pointer only once per function called, not once per usage therein. We
> could remove many of those fetches too, if we wanted to sprinkle the
> plperl code with yet more THX droppings; but I left that for another day.)
>
> Armed with that, I got rid of XSUB.h in plperl.c and moved the
> PG_TRY-using functions in the .xs files to plperl.c. I think this would
> fix Ashutosh's problem, though I am not in a position to try it with a
> PERL_IMPLICIT_SYS build here.

Thanks for the patch. I am seeing some compilation errors on Windows
with the patch. Below are the errors observed,

1>ClCompile:
1> plperl.c
1>src/pl/plperl/plperl.c(4051): error C2065: 'my_perl' : undeclared identifier

2>ClCompile:
2> hstore_plperl.c
2>contrib/hstore_plperl/hstore_plperl.c(77): error C2065: 'my_perl' :
undeclared identifier

I did spent some time to find reason for these compilation errors and
could eventually find that some of the Windows specific functions
inside plperl module are calling Perl APIs without fetching the perl
interpreter's context using dTHX. Please note that, we have now
defined PERL_NO_GET_CONTEXT macro before including the Perl headers in
plperl.h file which means any plperl function calling Perl APIs should
try to fetch the perl interpreter context variable 'my_perl' at the
start of the function using dTHX but, we were not doing that for
'setlocale_perl', 'hstore_to_plperl' and 'plperl_to_hstore' functions.
I have corrected this and attached is the new version of patch.

Moreover, I have also tested this patch along with the patch to import
switches used by perl into plperl and together it fixes the server
crash issue. Also, now, the interpreter size in both perl and plperl
module are the same thereby generating the same key on both plperl and
perl module. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com