Re: pg_waldump stucks with options --follow or -f and --stats or -z

On Sat, Nov 20, 2021 at 10:03:27AM +0530, Bharath Rupireddy wrote:
> Here's the v2 patch with the above changes i.e. pg_waldump now eimits
> the computed stats at the time of termination. Please review it.

+#define MAX_XLINFO_TYPES 16
+#define fatal_error(...) do { pg_log_fatal(__VA_ARGS__);
exit(EXIT_FAILURE); } while(0)
 typedef struct XLogDumpPrivate
Moving around those declarations does not relate to this patch, so I
would recommend to leave this out to ease reviews.

+   /*
+    * Although the pg_free calls (above and below) are unnecessary here on the
+    * process exit, let's not leak any memory knowingly.
+    */
+   pg_free(opts);
+
+   exit(EXIT_FAILURE);
There is no need to care about the two pg_free() calls here, so let's
remove all that.

+   pg_free(opts);
+   pg_free(stats);
+
    return EXIT_SUCCESS;
[...]
 bad_argument:
    fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
    progname);
+   pg_free(opts);
    return EXIT_FAILURE;
Same here.

+static void
+SignalHandlerForTermination(int signum)
+{
+   /* Display the stats only if they are valid */
+   if (stats &&
+       !XLogRecPtrIsInvalid(stats_startptr) &&
+       !XLogRecPtrIsInvalid(stats_endptr))
+   {
+       XLogDumpDisplayStats();
+       pg_free(stats);
+   }
+
+   /*
+    * Although the pg_free calls (above and below) are unnecessary here on the
+    * process exit, let's not leak any memory knowingly.
+    */
+   pg_free(opts);
+
+   exit(EXIT_FAILURE);
+}
Doing the dump of the stats followed by the exit() within the signal
handler callback is incorrect.  The only safe thing that can be set
and manipulated in a signal handler is roughly a sig_atomic_t flag.
What you should do is to set such a flag in the signal handler, and
then check its value in the main loop of pg_waldump, dumping the stats
if it is detected as turned on because of a signal.
--
Michael