Thread
Commits
-
Fix memory leak in BufFileCreateShared().
- 1cfdb1cb0e3f 11.0 landed
-
Fix memory leak.
- 969274d81301 11.0 landed
-
Add infrastructure for sharing temporary files between backends.
- dc6c4c9dc2a1 11.0 cited
-
Memory leaks in BufFileOpenShared()
Antonin Houska <ah@cybertec.at> — 2018-06-15T06:54:52Z
Memory is allocated twice for "file" and "files" variables. Possible fix: diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c index d8a18dd3dc..00f61748b3 100644 --- a/src/backend/storage/file/buffile.c +++ b/src/backend/storage/file/buffile.c @@ -277,10 +277,10 @@ BufFileCreateShared(SharedFileSet *fileset, const char *name) BufFile * BufFileOpenShared(SharedFileSet *fileset, const char *name) { - BufFile *file = (BufFile *) palloc(sizeof(BufFile)); + BufFile *file; char segment_name[MAXPGPATH]; Size capacity = 16; - File *files = palloc(sizeof(File) * capacity); + File *files; int nfiles = 0; file = (BufFile *) palloc(sizeof(BufFile)); -- Antonin Houska Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26, A-2700 Wiener Neustadt Web: https://www.cybertec-postgresql.com -
Re: Memory leaks in BufFileOpenShared()
Tatsuo Ishii <ishii@sraoss.co.jp> — 2018-06-15T07:20:03Z
> Memory is allocated twice for "file" and "files" variables. Possible fix: > > diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c > index d8a18dd3dc..00f61748b3 100644 > --- a/src/backend/storage/file/buffile.c > +++ b/src/backend/storage/file/buffile.c > @@ -277,10 +277,10 @@ BufFileCreateShared(SharedFileSet *fileset, const char *name) > BufFile * > BufFileOpenShared(SharedFileSet *fileset, const char *name) > { > - BufFile *file = (BufFile *) palloc(sizeof(BufFile)); > + BufFile *file; > char segment_name[MAXPGPATH]; > Size capacity = 16; > - File *files = palloc(sizeof(File) * capacity); > + File *files; > int nfiles = 0; > > file = (BufFile *) palloc(sizeof(BufFile)); Good catch. Thanks. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -
Re: Memory leaks in BufFileOpenShared()
Antonin Houska <ah@cybertec.at> — 2018-06-15T07:59:26Z
Now I see that BufFileCreateShared() has similar problem with file->name. More generic problem I see is that the common initialization of BufFile is repeated a few times. The attached patch tries to improve that (it also fixes the duplicate allocation of file->name). Tatsuo Ishii <ishii@sraoss.co.jp> wrote: > > Memory is allocated twice for "file" and "files" variables. Possible fix: > > > > diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c > > index d8a18dd3dc..00f61748b3 100644 > > --- a/src/backend/storage/file/buffile.c > > +++ b/src/backend/storage/file/buffile.c > > @@ -277,10 +277,10 @@ BufFileCreateShared(SharedFileSet *fileset, const char *name) > > BufFile * > > BufFileOpenShared(SharedFileSet *fileset, const char *name) > > { > > - BufFile *file = (BufFile *) palloc(sizeof(BufFile)); > > + BufFile *file; > > char segment_name[MAXPGPATH]; > > Size capacity = 16; > > - File *files = palloc(sizeof(File) * capacity); > > + File *files; > > int nfiles = 0; > > > > file = (BufFile *) palloc(sizeof(BufFile)); > > Good catch. Thanks. > > Best regards, > -- > Tatsuo Ishii > SRA OSS, Inc. Japan > English: http://www.sraoss.co.jp/index_en.php > Japanese:http://www.sraoss.co.jp > -- Antonin Houska Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26, A-2700 Wiener Neustadt Web: https://www.cybertec-postgresql.com -
Re: Memory leaks in BufFileOpenShared()
Tatsuo Ishii <ishii@sraoss.co.jp> — 2018-06-15T08:22:08Z
> Now I see that BufFileCreateShared() has similar problem with file->name. Right. > More generic problem I see is that the common initialization of BufFile is > repeated a few times. The attached patch tries to improve that (it also fixes > the duplicate allocation of file->name). The changes were made by this commit to add infrastructure for sharing temporary files between backends, according to the author (Thomas Munro). https://git.postgresql.org/pg/commitdiff/dc6c4c9dc2a111519b76b22daaaac86c5608223b Your proposal looks reasonable but I would like to hear from Thomas's opinion as well. Thomas? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
-
Re: Memory leaks in BufFileOpenShared()
Antonin Houska <ah@cybertec.at> — 2018-06-15T08:42:33Z
Tatsuo Ishii <ishii@sraoss.co.jp> wrote: > The changes were made by this commit to add infrastructure for sharing > temporary files between backends, according to the author (Thomas > Munro). > > https://git.postgresql.org/pg/commitdiff/dc6c4c9dc2a111519b76b22daaaac86c5608223b > > Your proposal looks reasonable but I would like to hear from Thomas's > opinion as well. o.k. He can actually have different feeling about details, e.g. if a new function makeBufFileCommon() should be introduced or if makeBufFile() should do the common settings. In the latter case, BufFileCreateTemp() would have to do more work than it does now. -- Antonin Houska Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26, A-2700 Wiener Neustadt Web: https://www.cybertec-postgresql.com
-
Re: Memory leaks in BufFileOpenShared()
Thomas Munro <thomas.munro@enterprisedb.com> — 2018-06-15T08:54:41Z
On Fri, Jun 15, 2018 at 8:42 PM, Antonin Houska <ah@cybertec.at> wrote: > Tatsuo Ishii <ishii@sraoss.co.jp> wrote: > >> The changes were made by this commit to add infrastructure for sharing >> temporary files between backends, according to the author (Thomas >> Munro). >> >> https://git.postgresql.org/pg/commitdiff/dc6c4c9dc2a111519b76b22daaaac86c5608223b >> >> Your proposal looks reasonable but I would like to hear from Thomas's >> opinion as well. > > o.k. He can actually have different feeling about details, e.g. if a new > function makeBufFileCommon() should be introduced or if makeBufFile() should > do the common settings. In the latter case, BufFileCreateTemp() would have to > do more work than it does now. Thanks for finding these accidental duplications, and to Ishii-san for committing the fix. Oops. +1 for this refactoring. -- Thomas Munro http://www.enterprisedb.com
-
Re: Memory leaks in BufFileOpenShared()
Tatsuo Ishii <ishii@sraoss.co.jp> — 2018-06-15T09:07:08Z
Hi Thomas, > On Fri, Jun 15, 2018 at 8:42 PM, Antonin Houska <ah@cybertec.at> wrote: >> Tatsuo Ishii <ishii@sraoss.co.jp> wrote: >> >>> The changes were made by this commit to add infrastructure for sharing >>> temporary files between backends, according to the author (Thomas >>> Munro). >>> >>> https://git.postgresql.org/pg/commitdiff/dc6c4c9dc2a111519b76b22daaaac86c5608223b >>> >>> Your proposal looks reasonable but I would like to hear from Thomas's >>> opinion as well. >> >> o.k. He can actually have different feeling about details, e.g. if a new >> function makeBufFileCommon() should be introduced or if makeBufFile() should >> do the common settings. In the latter case, BufFileCreateTemp() would have to >> do more work than it does now. > > Thanks for finding these accidental duplications, and to Ishii-san for > committing the fix. Oops. > > +1 for this refactoring. Thanks for confirming. I will go ahead commit/push the patch. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
-
Re: Memory leaks in BufFileOpenShared()
Tatsuo Ishii <ishii@sraoss.co.jp> — 2018-06-16T05:44:20Z
>> Thanks for finding these accidental duplications, and to Ishii-san for >> committing the fix. Oops. >> >> +1 for this refactoring. > > Thanks for confirming. I will go ahead commit/push the patch. Done. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp