On Thu, Oct 20, 2022 at 9:48 PM Robert Haas <robertmh...@gmail.com> wrote: > > On Thu, Oct 20, 2022 at 6:47 AM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: > > I tried implementing this, please see the attached v7 patch. > > I haven't checked this in detail but it looks much more reasonable in > terms of code footprint. However, we should, I think, set backup_state > = NULL and tablespace_map = NULL before deleting the memory context. > As you have it, I believe that if backup_state = (BackupState *) > palloc0(sizeof(BackupState)) fails -- say due to running out of memory > -- then those variables could end up pointing to garbage because the > context had already been reset before initializing them. I don't know > whether it's possible for that to cause any concrete harm, but nulling > out the pointers seems like cheap insurance.
I think elsewhere in the code we reset dangling pointers either ways - before or after deleting/resetting memory context. But placing them before would give us extra safety in case memory context deletion/reset fails. Not sure what's the best way. However, I'm nullifying the dangling pointers after deleting/resetting memory context. MemoryContextDelete(Conf->buildCxt); MemoryContextDelete(PostmasterContext); MemoryContextDelete(rulescxt); Please see the attached v8 patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
v8-0001-Avoid-memory-leaks-during-backups-using-SQL-calla.patch
Description: Binary data