Pavel Stehule <[email protected]> writes:
> attached patch allows raising exception from _PG_init function as was
> discussed before.
I fooled around with this and came up with the attached improved
version, which allows reporting the full error status. However,
after thinking some more I feel that this is probably a cure worse
than the disease. If we simply leave the code as it stands, an
elog(ERROR) in an init function doesn't corrupt dfmgr.c's internal list,
which is what I had been fearing when I complained about the issue.
The worst that happens is that we leave the library loaded and leak
a little bit of memory. Unloading the library, as the patch does,
could easily make things worse not better. Consider the not-unlikely
case that the library installs itself in a few callback hooks and
then fails. If we unload the library, those hooks represent
*guaranteed* core dumps on next use. If we don't unload, the hook
functions might or might not work too well --- presumably not everything
they need has been initialized --- but it's hard to imagine an outcome
that's worse than a guaranteed core dump.
So I'm thinking this is really unnecessary and we should leave well
enough alone.
regards, tom lane
Index: dfmgr.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/fmgr/dfmgr.c,v
retrieving revision 1.98
diff -c -r1.98 dfmgr.c
*** dfmgr.c 1 Jan 2009 17:23:51 -0000 1.98
--- dfmgr.c 1 Apr 2009 23:41:37 -0000
***************
*** 178,184 ****
static void *
internal_load_library(const char *libname)
{
! DynamicFileList *file_scanner;
PGModuleMagicFunction magic_func;
char *load_error;
struct stat stat_buf;
--- 178,184 ----
static void *
internal_load_library(const char *libname)
{
! DynamicFileList * volatile file_scanner;
PGModuleMagicFunction magic_func;
char *load_error;
struct stat stat_buf;
***************
*** 277,287 ****
}
/*
! * If the library has a _PG_init() function, call it.
*/
PG_init = (PG_init_t) pg_dlsym(file_scanner->handle,
"_PG_init");
if (PG_init)
! (*PG_init) ();
/* OK to link it into list */
if (file_list == NULL)
--- 277,329 ----
}
/*
! * If the library has a _PG_init() function, call it. Guard
against
! * the function possibly throwing elog(ERROR).
*/
PG_init = (PG_init_t) pg_dlsym(file_scanner->handle,
"_PG_init");
if (PG_init)
! {
! MemoryContext oldcontext = CurrentMemoryContext;
!
! PG_TRY();
! {
! (*PG_init) ();
! }
! PG_CATCH();
! {
! ErrorData *edata;
!
! /* fetch the error status so we can change it */
! MemoryContextSwitchTo(oldcontext);
! edata = CopyErrorData();
! FlushErrorState();
!
! /*
! * The const pointers in the error status very
likely point
! * at constant strings in the library, which we
are about to
! * unload. Copy them so we don't dump core
while reporting
! * the error. This might leak a bit of memory
but it
! * beats the alternatives.
! */
! if (edata->filename)
! edata->filename =
pstrdup(edata->filename);
! if (edata->funcname)
! edata->funcname =
pstrdup(edata->funcname);
! if (edata->domain)
! edata->domain = pstrdup(edata->domain);
!
! /* library might have already called some of
its functions */
!
clear_external_function_hash(file_scanner->handle);
!
! /* try to unlink library */
! pg_dlclose(file_scanner->handle);
! free((char *) file_scanner);
!
! /* complain */
! ReThrowError(edata);
! }
! PG_END_TRY();
! }
/* OK to link it into list */
if (file_list == NULL)
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
