Thread: BUG #3841: core dump in uuid-ossp
The following bug has been logged online: Bug reference: 3841 Logged by: Dmitriy Email address: im@ionflux.ru PostgreSQL version: 8.3.beta4 Operating system: FreeBSD 6.2-RELEASE Description: core dump in uuid-ossp Details: in sql console: postgres=# select uuid_nil(); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. on server console: LOG: server process (PID 41258) was terminated by signal 11: Segmentation fault LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server pro. HINT: In a moment you should be able to reconnect to the database and repeat your command. LOG: all server processes terminated; reinitializing LOG: database system was interrupted; last known up at 2007-12-27 11:53:03 IRKT LOG: database system was not properly shut down; automatic recovery in progress LOG: redo starts at 0/43FEA4 LOG: record with zero length at 0/447C94 LOG: redo done at 0/447C68 LOG: last completed transaction was at log time 2007-12-27 11:53:37.354834+08 gdb: [postgres@dev1 ~/data]$ gdb ../bin/postgres postgres.core GNU gdb 6.1.1 [FreeBSD] Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "i386-marcel-freebsd"... Core was generated by `postgres'. Program terminated with signal 11, Segmentation fault. Reading symbols from /usr/local/lib/libxslt.so.2...done. Loaded symbols for /usr/local/lib/libxslt.so.2 Reading symbols from /usr/local/lib/libxml2.so.5...done. Loaded symbols for /usr/local/lib/libxml2.so.5 Reading symbols from /usr/lib/libpam.so.3...done. Loaded symbols for /usr/lib/libpam.so.3 Reading symbols from /lib/libcrypt.so.3...done. Loaded symbols for /lib/libcrypt.so.3 Reading symbols from /lib/libm.so.4...done. Loaded symbols for /lib/libm.so.4 Reading symbols from /lib/libc.so.6...done. Loaded symbols for /lib/libc.so.6 Reading symbols from /lib/libz.so.3...done. Loaded symbols for /lib/libz.so.3 Reading symbols from /usr/local/lib/libiconv.so.3...done. Loaded symbols for /usr/local/lib/libiconv.so.3 Reading symbols from /usr/local/pgsql/lib/uuid-ossp.so...done. Loaded symbols for /usr/local/pgsql/lib/uuid-ossp.so Reading symbols from /libexec/ld-elf.so.1...done. Loaded symbols for /libexec/ld-elf.so.1 #0 0x2a5d5cd8 in uuid_import () from /usr/local/pgsql/lib/uuid-ossp.so (gdb) bt #0 0x2a5d5cd8 in uuid_import () from /usr/local/pgsql/lib/uuid-ossp.so #1 0x2a5d67d2 in uuid_load () from /usr/local/pgsql/lib/uuid-ossp.so #2 0x2a5d57aa in special_uuid_value (name=0x2a5dafa4 "nil") at uuid-ossp.c:95 #3 0x08141c34 in ExecMakeFunctionResult (fcache=0x1100bc8c, econtext=Cannot access memory at address 0xe89d9715 ) at execQual.c:1351 Cannot access memory at address 0xe89d970d (gdb) Quit
Dmitriy wrote: > in sql console: > postgres=# select uuid_nil(); > server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. Thanks for the report. I think the problem here is that we're not checking the return values of uuid functions. For example uuid_create could fail due to memory shortage or a number of other problems. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote: > Dmitriy wrote: > > > in sql console: > > postgres=# select uuid_nil(); > > server closed the connection unexpectedly > > This probably means the server terminated abnormally > > before or while processing the request. > > Thanks for the report. I think the problem here is that we're not > checking the return values of uuid functions. For example uuid_create > could fail due to memory shortage or a number of other problems. Please try the attached patch. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachment
Alvaro Herrera <alvherre@commandprompt.com> writes: > Please try the attached patch. Most of the invocations of pguuid_complain will be outright lies as to which function is complaining. Please rethink the error message. regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Please try the attached patch. > > Most of the invocations of pguuid_complain will be outright lies as to > which function is complaining. Please rethink the error message. Doh! Sorry. How about this: static void pguuid_complain(uuid_rc_t rc) { char *err = uuid_error(rc); if (err != NULL) ereport(ERROR, (errmsg("OSSP uuid failure: %s", err))); else ereport(ERROR, (errmsg("OSSP uuid failure: error code %d", rc))); } Alternatively we could pass the called function name into pguuid_complain, but I'm not sure it's worth the trouble (what does it give the user, anyway?) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes: > if (err != NULL) > ereport(ERROR, > (errmsg("OSSP uuid failure: %s", err))); > else > ereport(ERROR, > (errmsg("OSSP uuid failure: error code %d", rc))); Maybe "OSSP uuid library failure"? Otherwise seems OK. > Alternatively we could pass the called function name into > pguuid_complain, but I'm not sure it's worth the trouble (what does it > give the user, anyway?) Probably not much, if the uuid_error() strings are well written. Can we throw some more specific SQLSTATE than the default "internal error" here? Offhand I can't think of anything, but as a rule of thumb an ereport() ought to have an errcode(). If it really is an internal error then elog() is good enough. regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > if (err != NULL) > > ereport(ERROR, > > (errmsg("OSSP uuid failure: %s", err))); > > else > > ereport(ERROR, > > (errmsg("OSSP uuid failure: error code %d", rc))); > > Maybe "OSSP uuid library failure"? Otherwise seems OK. Sold. > > Alternatively we could pass the called function name into > > pguuid_complain, but I'm not sure it's worth the trouble (what does it > > give the user, anyway?) > > Probably not much, if the uuid_error() strings are well written. char *uuid_error(uuid_rc_t rc) { char *str; switch (rc) { case UUID_RC_OK: str = "everything ok"; break; case UUID_RC_ARG: str = "invalid argument"; break; case UUID_RC_MEM: str = "out of memory"; break; case UUID_RC_SYS: str = "system error"; break; case UUID_RC_INT: str = "internal error"; break; case UUID_RC_IMP: str = "not implemented"; break; default: str = NULL; break; } return str; } > Can we throw some more specific SQLSTATE than the default "internal > error" here? Offhand I can't think of anything, but as a rule of > thumb an ereport() ought to have an errcode(). If it really is an > internal error then elog() is good enough. Hmm. I looked at the extant list, and found that the contrib/xml2 code uses ERRCODE_EXTERNAL_ROUTINE_EXCEPTION whereas pgcrypto uses ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION. In passing, I noticed the use of xml_ereport and xml_ereport_by_code in the core XML code, and I'm left wondering whether we shouldn't add them to the list of gettext triggers in nls.mk. Also some messages there are marked with ERRCODE_INTERNAL_ERROR. (PLy_elog too?) (Sorry, totally unrelated:) Wow, in plpython.c there is this: static void * PLy_malloc(size_t bytes) { void *ptr = malloc(bytes); if (ptr == NULL) ereport(FATAL, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); return ptr; } I wonder why is it a good idea to have this report be FATAL. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes: > Tom Lane wrote: >> Can we throw some more specific SQLSTATE than the default "internal >> error" here? > Hmm. I looked at the extant list, and found that the contrib/xml2 code > uses ERRCODE_EXTERNAL_ROUTINE_EXCEPTION whereas pgcrypto uses > ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION. Either of those would be fine with me; it's not totally clear what distinction the SQL spec intends to draw. The best I can divine is that the latter is for bad arguments to the external routine, but we can't really tell whether that's applicable. > (Sorry, totally unrelated:) Wow, in plpython.c there is this: > ... > I wonder why is it a good idea to have this report be FATAL. Yeah, I noticed that recently and was annoyed by it, but I'm not sure how safe it is to change. I suspect the author was worried about leaked memory in event of a failure. regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > if (err != NULL) > > ereport(ERROR, > > (errmsg("OSSP uuid failure: %s", err))); > > else > > ereport(ERROR, > > (errmsg("OSSP uuid failure: error code %d", rc))); > > Maybe "OSSP uuid library failure"? Otherwise seems OK. Applied, thanks. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.