Fix some memory leaks associated with parsing json and manifests

Coverity complained about not freeing some memory associated with
incrementally parsing backup manifests. To fix that, provide and use a new
shutdown function for the JsonManifestParseIncrementalState object, in
line with a suggestion from Tom Lane.

While analysing the problem, I noticed a buglet in freeing memory for
incremental json lexers. To fix that remove a bogus condition on
freeing the memory allocated for them.

diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c
index 4962bf1529efc77ffc2c04cb7daae4606c23bf1a..330a22940116e9ba339d5e451a1f38d61743b91a 100644 (file)
--- a/src/backend/backup/basebackup_incremental.c
+++ b/src/backend/backup/basebackup_incremental.c
@@ -241,6 +241,9 @@ FinalizeIncrementalManifest(IncrementalBackupInfo *ib)
    pfree(ib->buf.data);
    ib->buf.data = NULL;
 
+   /* Done with inc_state, so release that memory too */
+   json_parse_manifest_incremental_shutdown(ib->inc_state);
+
    /* Switch back to previous memory context. */
    MemoryContextSwitchTo(oldcontext);
 }

diff --git a/src/bin/pg_combinebackup/load_manifest.c b/src/bin/pg_combinebackup/load_manifest.c
index 9c9332cdd5cfeb1856957183808215734c5b4772..d857ea00066196a3dd4c01c8f00d9ce1004c5526 100644 (file)
--- a/src/bin/pg_combinebackup/load_manifest.c
+++ b/src/bin/pg_combinebackup/load_manifest.c
@@ -208,6 +208,9 @@ load_backup_manifest(char *backup_directory)
                                                  inc_state, buffer, rc, bytes_left == 0);
        }
 
+       /* Release the incremental state memory */
+       json_parse_manifest_incremental_shutdown(inc_state);
+
        close(fd);
    }
 

diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 90ef4b20379aa252d5a4dd20470f1d363e570082..9594c615c7cb508fb4d1ec99a29af6c18f0b7d27 100644 (file)
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -484,6 +484,9 @@ parse_manifest_file(char *manifest_path)
                                                  inc_state, buffer, rc, bytes_left == 0);
        }
 
+       /* Release the incremental state memory */
+       json_parse_manifest_incremental_shutdown(inc_state);
+
        close(fd);
    }
 

diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 44dbb7f7f960c9d84fa1ef67910f11818c1586bb..9dfbc397c06d2cc36db346f7d93f71d114797304 100644 (file)
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -488,19 +488,18 @@ freeJsonLexContext(JsonLexContext *lex)
    if (lex->errormsg)
        destroyStringInfo(lex->errormsg);
 
-   if (lex->flags & JSONLEX_FREE_STRUCT)
+   if (lex->incremental)
    {
-       if (lex->incremental)
-       {
-           pfree(lex->inc_state->partial_token.data);
-           pfree(lex->inc_state);
-           pfree(lex->pstack->prediction);
-           pfree(lex->pstack->fnames);
-           pfree(lex->pstack->fnull);
-           pfree(lex->pstack);
-       }
-       pfree(lex);
+       pfree(lex->inc_state->partial_token.data);
+       pfree(lex->inc_state);
+       pfree(lex->pstack->prediction);
+       pfree(lex->pstack->fnames);
+       pfree(lex->pstack->fnull);
+       pfree(lex->pstack);
    }
+
+   if (lex->flags & JSONLEX_FREE_STRUCT)
+       pfree(lex);
 }
 
 /*

diff --git a/src/common/parse_manifest.c b/src/common/parse_manifest.c
index 970a756ce8ad4967d4f200385e7eecd095b1e051..a94e3d6b1549375e05ff57ae6ce46985c6cfca1d 100644 (file)
--- a/src/common/parse_manifest.c
+++ b/src/common/parse_manifest.c
@@ -123,7 +123,6 @@ static bool parse_xlogrecptr(XLogRecPtr *result, char *input);
 
 /*
  * Set up for incremental parsing of the manifest.
- *
  */
 
 JsonManifestParseIncrementalState *
@@ -163,6 +162,18 @@ json_parse_manifest_incremental_init(JsonManifestParseContext *context)
    return incstate;
 }
 
+/*
+ * Free an incremental state object and its contents.
+ */
+void
+json_parse_manifest_incremental_shutdown(JsonManifestParseIncrementalState *incstate)
+{
+   pfree(incstate->sem.semstate);
+   freeJsonLexContext(&(incstate->lex));
+   /* incstate->manifest_ctx has already been freed */
+   pfree(incstate);
+}
+
 /*
  * parse the manifest in pieces.
  *

diff --git a/src/include/common/parse_manifest.h b/src/include/common/parse_manifest.h
index 3aa594fcac76853f41e570fdf7cb1f00bb29ad88..0d04239c38dda5fa8cda763fca82c6f8cdb0bf59 100644 (file)
--- a/src/include/common/parse_manifest.h
+++ b/src/include/common/parse_manifest.h
@@ -53,5 +53,6 @@ extern JsonManifestParseIncrementalState *json_parse_manifest_incremental_init(J
 extern void json_parse_manifest_incremental_chunk(
                                                  JsonManifestParseIncrementalState *incstate, char *chunk, int size,
                                                  bool is_last);
+extern void json_parse_manifest_incremental_shutdown(JsonManifestParseIncrementalState *incstate);
 
 #endif