Repair an error introduced by log_line_prefix patch: it is not acceptable

to assume that the string pointer passed to set_ps_display is good forever.
There's no need to anyway since ps_status.c itself saves the string, and
we already had an API (get_ps_display) to return it.
I believe this explains Jim Nasby's report of intermittent crashes in
elog.c when %i format code is in use in log_line_prefix.
While at it, repair a previously unnoticed problem: on some platforms such as
Darwin, the string returned by get_ps_display was blank-padded to the maximum
length, meaning that lock.c's attempt to append " waiting" to it never worked.
This commit is contained in:
Tom Lane 2005-11-05 03:04:53 +00:00
parent 95af2633c3
commit 48052de722
6 changed files with 43 additions and 26 deletions

View File

@ -37,7 +37,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.474 2005/11/03 20:02:50 alvherre Exp $ * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.475 2005/11/05 03:04:52 tgl Exp $
* *
* NOTES * NOTES
* *
@ -2647,7 +2647,6 @@ BackendRun(Port *port)
/* set these to empty in case they are needed before we set them up */ /* set these to empty in case they are needed before we set them up */
port->remote_host = ""; port->remote_host = "";
port->remote_port = ""; port->remote_port = "";
port->commandTag = "";
/* /*
* Initialize libpq and enable reporting of ereport errors to the client. * Initialize libpq and enable reporting of ereport errors to the client.

View File

@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.158 2005/10/15 02:49:26 momjian Exp $ * $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.159 2005/11/05 03:04:52 tgl Exp $
* *
* NOTES * NOTES
* Outside modules can create a lock table and acquire/release * Outside modules can create a lock table and acquire/release
@ -1049,21 +1049,21 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
ResourceOwner owner) ResourceOwner owner)
{ {
LockMethod lockMethodTable = LockMethods[lockmethodid]; LockMethod lockMethodTable = LockMethods[lockmethodid];
char *new_status, const char *old_status;
*old_status; char *new_status;
size_t len; int len;
Assert(lockmethodid < NumLockMethods); Assert(lockmethodid < NumLockMethods);
LOCK_PRINT("WaitOnLock: sleeping on lock", LOCK_PRINT("WaitOnLock: sleeping on lock",
locallock->lock, locallock->tag.mode); locallock->lock, locallock->tag.mode);
old_status = pstrdup(get_ps_display()); old_status = get_ps_display(&len);
len = strlen(old_status);
new_status = (char *) palloc(len + 8 + 1); new_status = (char *) palloc(len + 8 + 1);
memcpy(new_status, old_status, len); memcpy(new_status, old_status, len);
strcpy(new_status + len, " waiting"); strcpy(new_status + len, " waiting");
set_ps_display(new_status); set_ps_display(new_status);
new_status[len] = '\0'; /* truncate off " waiting" */
awaitedLock = locallock; awaitedLock = locallock;
awaitedOwner = owner; awaitedOwner = owner;
@ -1104,8 +1104,7 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
awaitedLock = NULL; awaitedLock = NULL;
set_ps_display(old_status); set_ps_display(new_status);
pfree(old_status);
pfree(new_status); pfree(new_status);
LOCK_PRINT("WaitOnLock: wakeup on lock", LOCK_PRINT("WaitOnLock: wakeup on lock",

View File

@ -42,7 +42,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.166 2005/11/03 17:11:39 alvherre Exp $ * $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.167 2005/11/05 03:04:52 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -67,6 +67,7 @@
#include "tcop/tcopprot.h" #include "tcop/tcopprot.h"
#include "utils/memutils.h" #include "utils/memutils.h"
#include "utils/guc.h" #include "utils/guc.h"
#include "utils/ps_status.h"
/* Global variables */ /* Global variables */
@ -1484,19 +1485,26 @@ log_line_prefix(StringInfo buf)
break; break;
case 'i': case 'i':
if (MyProcPort) if (MyProcPort)
appendStringInfo(buf, "%s", MyProcPort->commandTag); {
const char *psdisp;
int displen;
psdisp = get_ps_display(&displen);
appendStringInfo(buf, "%.*s", displen, psdisp);
}
break; break;
case 'r': case 'r':
if (MyProcPort) if (MyProcPort && MyProcPort->remote_host)
{ {
appendStringInfo(buf, "%s", MyProcPort->remote_host); appendStringInfo(buf, "%s", MyProcPort->remote_host);
if (strlen(MyProcPort->remote_port) > 0) if (MyProcPort->remote_port &&
MyProcPort->remote_port[0] != '\0')
appendStringInfo(buf, "(%s)", appendStringInfo(buf, "(%s)",
MyProcPort->remote_port); MyProcPort->remote_port);
} }
break; break;
case 'h': case 'h':
if (MyProcPort) if (MyProcPort && MyProcPort->remote_host)
appendStringInfo(buf, "%s", MyProcPort->remote_host); appendStringInfo(buf, "%s", MyProcPort->remote_host);
break; break;
case 'q': case 'q':

View File

@ -5,7 +5,7 @@
* to contain some useful information. Mechanism differs wildly across * to contain some useful information. Mechanism differs wildly across
* platforms. * platforms.
* *
* $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.25 2005/10/15 02:49:36 momjian Exp $ * $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.26 2005/11/05 03:04:52 tgl Exp $
* *
* Copyright (c) 2000-2005, PostgreSQL Global Development Group * Copyright (c) 2000-2005, PostgreSQL Global Development Group
* various details abducted from various places * various details abducted from various places
@ -307,10 +307,6 @@ init_ps_display(const char *username, const char *dbname,
void void
set_ps_display(const char *activity) set_ps_display(const char *activity)
{ {
/* save tag for possible use by elog.c */
if (MyProcPort)
MyProcPort->commandTag = activity;
#ifndef PS_USE_NONE #ifndef PS_USE_NONE
/* no ps display for stand-alone backend */ /* no ps display for stand-alone backend */
if (!IsUnderPostmaster) if (!IsUnderPostmaster)
@ -365,15 +361,31 @@ set_ps_display(const char *activity)
/* /*
* Returns what's currently in the ps display, in case someone needs * Returns what's currently in the ps display, in case someone needs
* it. Note that only the activity part is returned. * it. Note that only the activity part is returned. On some platforms
* the string will not be null-terminated, so return the effective
* length into *displen.
*/ */
const char * const char *
get_ps_display(void) get_ps_display(int *displen)
{ {
#ifdef PS_USE_CLOBBER_ARGV #ifdef PS_USE_CLOBBER_ARGV
size_t offset;
/* If ps_buffer is a pointer, it might still be null */ /* If ps_buffer is a pointer, it might still be null */
if (!ps_buffer) if (!ps_buffer)
{
*displen = 0;
return ""; return "";
}
/* Remove any trailing spaces to offset the effect of PS_PADDING */
offset = ps_buffer_size;
while (offset > ps_buffer_fixed_size && ps_buffer[offset-1] == PS_PADDING)
offset--;
*displen = offset - ps_buffer_fixed_size;
#else
*displen = strlen(ps_buffer + ps_buffer_fixed_size);
#endif #endif
return ps_buffer + ps_buffer_fixed_size; return ps_buffer + ps_buffer_fixed_size;

View File

@ -11,7 +11,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.52 2005/10/15 02:49:44 momjian Exp $ * $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.53 2005/11/05 03:04:53 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -80,7 +80,6 @@ typedef struct Port
* but since it gets used by elog.c in the same way as database_name and * but since it gets used by elog.c in the same way as database_name and
* other members of this struct, we may as well keep it here. * other members of this struct, we may as well keep it here.
*/ */
const char *commandTag; /* current command tag */
struct timeval session_start; /* for session duration logging */ struct timeval session_start; /* for session duration logging */
/* /*

View File

@ -4,7 +4,7 @@
* *
* Declarations for backend/utils/misc/ps_status.c * Declarations for backend/utils/misc/ps_status.c
* *
* $PostgreSQL: pgsql/src/include/utils/ps_status.h,v 1.25 2004/02/22 21:26:54 tgl Exp $ * $PostgreSQL: pgsql/src/include/utils/ps_status.h,v 1.26 2005/11/05 03:04:53 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -19,6 +19,6 @@ extern void init_ps_display(const char *username, const char *dbname,
extern void set_ps_display(const char *activity); extern void set_ps_display(const char *activity);
extern const char *get_ps_display(void); extern const char *get_ps_display(int *displen);
#endif /* PS_STATUS_H */ #endif /* PS_STATUS_H */