Re: Dereference before NULL check (src/backend/storage/ipc/latch.c) - Mailing list pgsql-hackers

At Mon, 02 Nov 2020 17:25:04 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> I called SetLatch() during WaitLatch(NULL, ) but that doesn't fire
> WL_LATCH_SET event for me on Windows. (I got it fired on Linux..)  On
> Windows, the latch is detected after exiting the WaitLatch()
> call. Seems like MyLatch of waiter is different from
> peerPGPROC->procLatch.  And... an update for Visual Studio broke my
> environment... I will investigate this further but everything feel
> cumbersome on Windows...

I managed to reproduce the issue. FWIW the attached modifies
pg_backend_pid() to call "WaitLatch(NULL," and
pg_terminate_backend(pid) to SetLatch() to the process latch of the
pid.  (It's minunderstanding that I could reproduce this on Linux.)

Session A:
  =# select pg_backend_pid();  -- sleeps for 10 seconds.

Session B:
  =# select pg_terminate_backend(A-pid);

[11628] LOG:  server process (PID 14568) was terminated by exception 0xC0000005
[11628] DETAIL:  Failed process was running: select pg_backend_pid();
[11628] HINT:  See C include file "ntstatus.h" for a description of the hexadecimal value.
[11628] LOG:  terminating any other active server processes
[2948] WARNING:  terminating connection because of crash of another server process
2

With the fix patch, it changes to:

[16632] LOG:  FALSE LATCH: 0000000000000000

rebards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index 24d44c98..e900bd3c 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -1845,6 +1845,8 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout,
             occurred_events++;
             returned_events++;
         }
+        else if (!set->latch)
+          elog(LOG, "FALSE LATCH: %p", set->latch);
     }
     else if (cur_event->events == WL_POSTMASTER_DEATH)
     {
diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c
index d822e82c..b7340bf4 100644
--- a/src/backend/storage/ipc/signalfuncs.c
+++ b/src/backend/storage/ipc/signalfuncs.c
@@ -134,7 +134,13 @@ pg_cancel_backend(PG_FUNCTION_ARGS)
 Datum
 pg_terminate_backend(PG_FUNCTION_ARGS)
 {
-    int            r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM);
+    int r = 0;
+    PGPROC *p = BackendPidGetProc(PG_GETARG_INT32(0));
+    
+    if (p)
+        SetLatch(&p->procLatch);
+
+    PG_RETURN_BOOL(false);
 
     if (r == SIGNAL_BACKEND_NOSUPERUSER)
         ereport(ERROR,
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index a210fc93..1e2b97a9 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -960,6 +960,9 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 Datum
 pg_backend_pid(PG_FUNCTION_ARGS)
 {
+    WaitLatch(MyLatch, WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, 1, WAIT_EVENT_RECOVERY_PAUSE);
+    WaitLatch(NULL, WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, 10000, WAIT_EVENT_RECOVERY_PAUSE);
+
     PG_RETURN_INT32(MyProcPid);
 }