Re: help to identify the reason that extension's C function returns array get segmentation fault

=?UTF-8?B?6ZKx5paw5p6X?= <qianxinlin(at)gmail(dot)com> writes:
> I have written an extension to manage openstreetmap data. There is a C
> function to perform spatial top k query on several tables and return an
> array of int8 type as result. The code skeleton of this function is as
> follows:

There are a remarkable lot of bugs in this code fragment. Many of them
would not bite you as long as you are running on 64-bit Intel hardware,
but that doesn't make them not bugs.

> Datum vquery(PG_FUNCTION_ARGS) {

> int array_len = PG_GETARG_INT32(0);
> long * node_ids;

> SPI_connect();

> //some code to retrieve data from various tables
> // node_ids are allocated and filled up

> ArrayType * retarr;
> Datum * vals ;

> vals = palloc0(array_len * sizeof(long));

Datum is not necessarily the same as "long".

> // fill the vals up
> for (i = 0 ; i < array_len ; i++)
> vals[i] = Int64GetDatum((node_ids[i]));

int64 is not necessarily the same as "long", either.

> retarr = construct_array(vals, retcnt, INT8OID, sizeof(long), true, 'i');

Again, INT8 is not the same size as "long", and it's not necessarily
pass-by-val, and it's *certainly* not integer alignment.

> SPI_finish();

> PG_RETURN_ARRAYTYPE_P(retarr);

But I think what's really biting you, probably, is that construct_array()
made the array in CurrentMemoryContext which at that point was the SPI
execution context; which would be deleted by SPI_finish. So you're
returning a dangling pointer. You need to do something to either copy
the array value out to the caller's context, or build it there in the
first place.

BTW, this failure would be a lot less intermittent if you were testing
in a CLOBBER_FREED_MEMORY build. I would go so far as to say you should
*never* develop or test C code for the Postgres backend without using
the --enable-cassert configure option for your build. You're simply
tossing away a whole lot of debug support if you don't.

regards, tom lane