2009/4/2 Tom Lane <[email protected]>:
> 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.
>
I see it. I thing , an safety of this exception should be solved only
by programmer. It's important to release all hooks, and then raise an
exception. It is in developer responsibility.
regards
Pavel Stehule
> 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
