Thread

1. Fix memory leak in tzparser.c

   Shixin Wang <wang-shi-xin@outlook.com> — 2025-12-16T05:55:32Z

   Hi hackers,
   
   I noticed a memory leak in the addToArray() function in src/backend/utils/misc/tzparser.c.
   When the override parameter is true and a duplicate timezone abbreviation is found,
   the code overwrites midptr->zone without freeing the previously allocated memory.
   
   The fix would be:
   
   -               midptr->zone = entry->zone;
   +               if (midptr->zone != NULL)
   +                   pfree(midptr->zone);
   +               midptr->zone = entry->zone;
   
   While the memory is managed by a temp memory context that gets cleaned up
   eventually, the coarse-grained management might cause some memory to
   accumulate during ParseTzFile() recursive calls when processing @INCLUDE
   directives.
   
   I've attached a patch with this change in case anyone thinks it's worth
   applying.
   
   Regards,
   Shixin Wang
   
   
   
   
   

2. Re: Fix memory leak in tzparser.c

   Michael Paquier <michael@paquier.xyz> — 2025-12-16T07:59:35Z

   On Tue, Dec 16, 2025 at 05:55:32AM +0000, Shixin Wang wrote:
   > While the memory is managed by a temp memory context that gets cleaned up
   > eventually, the coarse-grained management might cause some memory to
   > accumulate during ParseTzFile() recursive calls when processing @INCLUDE
   > directives.
   > 
   > I've attached a patch with this change in case anyone thinks it's worth
   > applying.
   
   Why does it matter?  load_tzoffsets() is the sole caller of
   ParseTzFile() and it uses a temporary memory context named
   TZParserMemory to not have to do cleanups like the one you are
   proposing here.
   --
   Michael
   

3. Re: Fix memory leak in tzparser.c

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-12-17T13:17:47Z

   On Tue, Dec 16, 2025 at 1:29 PM Michael Paquier <michael@paquier.xyz> wrote:
   >
   > On Tue, Dec 16, 2025 at 05:55:32AM +0000, Shixin Wang wrote:
   > > While the memory is managed by a temp memory context that gets cleaned up
   > > eventually, the coarse-grained management might cause some memory to
   > > accumulate during ParseTzFile() recursive calls when processing @INCLUDE
   > > directives.
   > >
   > > I've attached a patch with this change in case anyone thinks it's worth
   > > applying.
   >
   > Why does it matter?  load_tzoffsets() is the sole caller of
   > ParseTzFile() and it uses a temporary memory context named
   > TZParserMemory to not have to do cleanups like the one you are
   > proposing here.
   
   +1. But maybe Shixin has seen a scenario where this temporary
   accumulation has caused some problems because say there were many
   entries whose zone was replaced? Shixin, what problem did you see
   which prompted you to create this patch?
   
   -- 
   Best Wishes,
   Ashutosh Bapat
   
   
   
   

4. Re: Fix memory leak in tzparser.c

   Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-17T15:49:05Z

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
   > On Tue, Dec 16, 2025 at 1:29 PM Michael Paquier <michael@paquier.xyz> wrote:
   >> Why does it matter?  load_tzoffsets() is the sole caller of
   >> ParseTzFile() and it uses a temporary memory context named
   >> TZParserMemory to not have to do cleanups like the one you are
   >> proposing here.
   
   > +1. But maybe Shixin has seen a scenario where this temporary
   > accumulation has caused some problems because say there were many
   > entries whose zone was replaced? Shixin, what problem did you see
   > which prompted you to create this patch?
   
   There are only several hundred timezone names in the entire world,
   so it's really difficult to believe any interesting amount of
   transient memory consumption here.
   
   			regards, tom lane