[PATCH] Fix memory leak of primary_sysid in walreceiver

  • Jump to comment-1
    DaeMyung Kang<charsyam@gmail.com>
    Apr 26, 2026, 5:02 PM UTC
    Hi, Hackers,
    
    In WalReceiverMain(), the outer streaming loop calls
    walrcv_identify_system() once per iteration to verify the primary's
    system identifier:
    
      primary_sysid = walrcv_identify_system(wrconn, &primaryTLI);
      ...
      if (strcmp(primary_sysid, standby_sysid) != 0)
          ereport(ERROR, ...);
    
    walrcv_identify_system() (libpqrcv_identify_system() in
    libpqwalreceiver.c) returns a pstrdup()'d string, but the caller
    never frees it. Each streaming restart therefore leaks the string.
    The error path is unaffected because the surrounding memory context
    is reset on ERROR.
    
    The attached patch adds a pfree(primary_sysid) right after the
    comparison.
    
    This dates back to commit 78c8c814390 ("Refactor libpqwalreceiver",
    2016), so it should be a back-patch candidate as well.
    
    No new tests are added; the fix only releases resources and does not
    change observable behavior. `make check` and the streaming replication
    TAP test (src/test/recovery/t/001_stream_rep.pl) pass with the patch
    applied.
    
    Patch attached.
    
    Regards,
    DaeMyung Kang
    • Jump to comment-1
      Michael Paquier<michael@paquier.xyz>
      Apr 27, 2026, 12:37 AM UTC
      On Mon, Apr 27, 2026 at 02:01:30AM +0900, DaeMyung Kang wrote:
      The attached patch adds a pfree(primary_sysid) right after the
      comparison.
      My first impression was that it does not matter because I was under
      the impression that this code path is only reached once, but that's
      not the case: a WAL receiver could stay around waiting for
      instructions before retrying a connection.
      I doubt that this is worth bothering for in the back branches, as it
      just means a small leak each time we switch to a new TLI repeatedly,
      something that would matter mostly for a cascading standby where we
      don't want to change the connection point (a SIGHUP'd primary_conninto
      enforces a WAL receiver shutdown). Let's clean up on HEAD, though.
      --
      Michael