Thread: Re: [GENERAL] backend crashing despite tsearch2 patch

On Wed, 17 Sep 2003 psql-mail@freeuk.com wrote:

> > On Wed, 17 Sep 2003 psql-mail@freeuk.com wrote:
> >
> > > I have applied the recent tsearch2 patch and recompiled the
> tsearch2
> > > module but I am still experiencing the same backend crashes as I
> > > previously described.
> >
> > I didn't think your problem was the same as mine.
> >
> > > #0  SN_create_env (S_size=0, I_size=2, B_size=1) at api.c:6
> > > 6           z->p = create_s();
> > > (gdb) bt
> > > #0  SN_create_env (S_size=0, I_size=2, B_size=1) at api.c:6
> > > #1  0x20000000026be870 in SN_create_env (S_size=40770504, I_size=
> > > 40509856,
> > > B_size=1034) at api.c:6
> >
> > Is that the full backtrace?
>
> The gdb session above is quoted above start to finish as displayed on
> screen. I'm not very famialiar with gdb so please say if I need to do
> things differently.
>
> So i think it is the full backtrace - i certainly haven't edited
> anything.


Trouble is it doesn't look like a decently deep stack. I would have expected to
see a lot more output from the backtrace.

Having said that the z in the z->p = create_s() line mentioned as the place of
the fault is the result of a calloc without checking for a null return from
calloc. Here's a[nother simple] patch to fix that.

It's not going to fix whatever is putting you into the situation that makes
calloc fail though. It'll just make the failure less disasterous.


--
Nigel J. Andrews

This replaces the previous patch in this thread.


On Wed, 17 Sep 2003, Nigel J. Andrews wrote:
>
> On Wed, 17 Sep 2003 psql-mail@freeuk.com wrote:
>
> > > On Wed, 17 Sep 2003 psql-mail@freeuk.com wrote:
> > >
> > > > #0  SN_create_env (S_size=0, I_size=2, B_size=1) at api.c:6
> > > > 6           z->p = create_s();
> > > > (gdb) bt
> > > > #0  SN_create_env (S_size=0, I_size=2, B_size=1) at api.c:6
> > > > #1  0x20000000026be870 in SN_create_env (S_size=40770504, I_size=
> > > > 40509856,
> > > > B_size=1034) at api.c:6
> > >
> ...
> Having said that the z in the z->p = create_s() line mentioned as the place of
> the fault is the result of a calloc without checking for a null return from
> calloc. Here's a[nother simple] patch to fix that.
>
> It's not going to fix whatever is putting you into the situation that makes
> calloc fail though. It'll just make the failure less disasterous.


Here's a slightly more paranoid patch, i.e. it checks all the allocations done
in the routine instead of just the specific instance from the stack trace the
previous patch checked.

On a matter of style, it's been a while since I've seriously considered cross
platform C. Is it the done thing to expect:

 int *i = (int *)calloc(1,sizeof(int));

to give the condition *i == 0 (assuming the memory allocation worked)?

Also, I've not tested the amended code since I don't know that much about the
configuration of tsearch2 and specifically what this snowball stuff
is. However, it builds for me and a test query didn't break. I'd appreciate if
someone could give the changes a quick once over for correctness and if someone
could actually test this (maybe pgsql-mail@freeuk.com). In the meantime I'll
see if I can get the regression test to run.

--
Nigel J. Andrews

On Wed, 17 Sep 2003, Nigel J. Andrews wrote:

> Also, I've not tested the amended code since I don't know that much about the
> configuration of tsearch2 and specifically what this snowball stuff
> is. However, it builds for me and a test query didn't break. I'd appreciate if
> someone could give the changes a quick once over for correctness and if someone
> could actually test this (maybe pgsql-mail@freeuk.com). In the meantime I'll
> see if I can get the regression test to run.

The installcheck passes.


--
Nigel J. Andrews

"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
> On a matter of style, it's been a while since I've seriously considered cross
> platform C. Is it the done thing to expect:
>  int *i = (int *)calloc(1,sizeof(int));
> to give the condition *i == 0 (assuming the memory allocation worked)?

calloc is defined to zero out the block of memory it returns (as opposed
to malloc which may return a block containing any random junk).

A more serious question is whether any of this code should be using
calloc/malloc as opposed to palloc.  I'd prefer to see it rewritten to
use palloc wherever possible; but that begs the question of what the
required lifespan of the allocations is.

+ #ifndef NULL
+ #define NULL ((void *)0)
+ #endif

It has been roughly twenty years since a C platform existed that didn't
predefine NULL ... and the ones that did not would likely not recognize
the ANSI-C-ism "void *".  So the above is unhelpful by any measure.

            regards, tom lane