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.

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b780ec4a1204c1adea4f3b8efadadffd50ae2be6..82380f1b0874979ffc68e910a5cae2aa25033769 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2647,7 +2647,6 @@ BackendRun(Port *port)
        /* set these to empty in case they are needed before we set them up */
        port->remote_host = "";
        port->remote_port = "";
-       port->commandTag = "";
 
        /*
         * Initialize libpq and enable reporting of ereport errors to the client.

diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 9c3b286f712e3c4bb86a94f6d2d173d9b2d7722a..70548995a0f7b5f7c523cb9aa0e4f8b483bc9943 100644 (file)
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -1049,21 +1049,21 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
                   ResourceOwner owner)
 {
        LockMethod      lockMethodTable = LockMethods[lockmethodid];
-       char       *new_status,
-                          *old_status;
-       size_t          len;
+       const char *old_status;
+       char       *new_status;
+       int                     len;
 
        Assert(lockmethodid < NumLockMethods);
 
        LOCK_PRINT("WaitOnLock: sleeping on lock",
                           locallock->lock, locallock->tag.mode);
 
-       old_status = pstrdup(get_ps_display());
-       len = strlen(old_status);
+       old_status = get_ps_display(&len);
        new_status = (char *) palloc(len + 8 + 1);
        memcpy(new_status, old_status, len);
        strcpy(new_status + len, " waiting");
        set_ps_display(new_status);
+       new_status[len] = '\0';         /* truncate off " waiting" */
 
        awaitedLock = locallock;
        awaitedOwner = owner;
@@ -1104,8 +1104,7 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
 
        awaitedLock = NULL;
 
-       set_ps_display(old_status);
-       pfree(old_status);
+       set_ps_display(new_status);
        pfree(new_status);
 
        LOCK_PRINT("WaitOnLock: wakeup on lock",

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 42110876a7fc8a52404eb45d062fe590b8e17411..149c2480e63744661ab445bab38ffcebcc8dc6f4 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -67,6 +67,7 @@
 #include "tcop/tcopprot.h"
 #include "utils/memutils.h"
 #include "utils/guc.h"
+#include "utils/ps_status.h"
 
 
 /* Global variables */
@@ -1484,19 +1485,26 @@ log_line_prefix(StringInfo buf)
                                break;
                        case 'i':
                                if (MyProcPort)
-                                       appendStringInfo(buf, "%s", MyProcPort->commandTag);
+                               {
+                                       const char *psdisp;
+                                       int             displen;
+
+                                       psdisp = get_ps_display(&displen);
+                                       appendStringInfo(buf, "%.*s", displen, psdisp);
+                               }
                                break;
                        case 'r':
-                               if (MyProcPort)
+                               if (MyProcPort && 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)",
                                                                                 MyProcPort->remote_port);
                                }
                                break;
                        case 'h':
-                               if (MyProcPort)
+                               if (MyProcPort && MyProcPort->remote_host)
                                        appendStringInfo(buf, "%s", MyProcPort->remote_host);
                                break;
                        case 'q':

diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index f55da3839d1c9e2e2281c39c92daccd45d41f95d..d820a91674a3c25ec367ebc5097bf157ed12db63 100644 (file)
--- a/src/backend/utils/misc/ps_status.c
+++ b/src/backend/utils/misc/ps_status.c
@@ -307,10 +307,6 @@ init_ps_display(const char *username, const char *dbname,
 void
 set_ps_display(const char *activity)
 {
-       /* save tag for possible use by elog.c */
-       if (MyProcPort)
-               MyProcPort->commandTag = activity;
-
 #ifndef PS_USE_NONE
        /* no ps display for stand-alone backend */
        if (!IsUnderPostmaster)
@@ -365,15 +361,31 @@ set_ps_display(const char *activity)
 
 /*
  * 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 *
-get_ps_display(void)
+get_ps_display(int *displen)
 {
 #ifdef PS_USE_CLOBBER_ARGV
+       size_t          offset;
+
        /* If ps_buffer is a pointer, it might still be null */
        if (!ps_buffer)
+       {
+               *displen = 0;
                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
 
        return ps_buffer + ps_buffer_fixed_size;

diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 970ad5f2328319ef1c616c0c0a4a920bb52226f7..4f59423998533902237a793de52388d8526f951e 100644 (file)
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -80,7 +80,6 @@ typedef struct Port
         * 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.
         */
-       const char *commandTag;         /* current command tag */
        struct timeval session_start;           /* for session duration logging */
 
        /*

diff --git a/src/include/utils/ps_status.h b/src/include/utils/ps_status.h
index 122cd94915f64f9782d4e17bb987e92696a25753..20eb6446bec58a8b011e664c26b78cf7b210d7fa 100644 (file)
--- a/src/include/utils/ps_status.h
+++ b/src/include/utils/ps_status.h
@@ -19,6 +19,6 @@ extern void init_ps_display(const char *username, const char *dbname,
 
 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 */