Lists: | pgsql-hackers |
---|
From: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Fixing memory leak in pg_upgrade |
Date: | 2015-01-09 12:23:50 |
Message-ID: | 20150109.212350.1299395873280569132.t-ishii@sraoss.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
According to Coverity, there's a memory leak bug in transfer_all_new_dbs().
mappings = gen_db_file_maps(old_db, new_db, &n_maps, old_pgdata,
new_pgdata);
if (n_maps)
{
print_maps(mappings, n_maps, new_db->db_name);
#ifdef PAGE_CONVERSION
pageConverter = setupPageConverter();
#endif
transfer_single_new_db(pageConverter, mappings, n_maps,
old_tablespace);
pg_free(mappings);
}
-----> leaks "mappings"
}
return;
}
This is because gen_db_file_maps() allocates memory even if n_maps == 0.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fixing memory leak in pg_upgrade |
Date: | 2015-01-09 12:38:35 |
Message-ID: | CAB7nPqRD=bVXGKn6k61mJj4TAaJi0W_MMBrmz1JnBC91VsieVQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Fri, Jan 9, 2015 at 9:23 PM, Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
> This is because gen_db_file_maps() allocates memory even if n_maps == 0.
Purely cosmetic: the initialization n_maps = 0 before the call of
gen_db_file_maps is unnecessary ;)
--
Michael
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fixing memory leak in pg_upgrade |
Date: | 2015-01-09 16:34:24 |
Message-ID: | 7341.1420821264@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tatsuo Ishii <ishii(at)postgresql(dot)org> writes:
> According to Coverity, there's a memory leak bug in transfer_all_new_dbs().
It's pretty difficult to get excited about that; how many table-free
databases is pg_upgrade likely to see in one run? But surely we could
just move the pg_free call to after the if-block.
regards, tom lane
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Tatsuo Ishii <ishii(at)postgresql(dot)org>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fixing memory leak in pg_upgrade |
Date: | 2015-01-09 17:27:59 |
Message-ID: | 20150109172759.GB26812@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Fri, Jan 9, 2015 at 11:34:24AM -0500, Tom Lane wrote:
> Tatsuo Ishii <ishii(at)postgresql(dot)org> writes:
> > According to Coverity, there's a memory leak bug in transfer_all_new_dbs().
>
> It's pretty difficult to get excited about that; how many table-free
> databases is pg_upgrade likely to see in one run? But surely we could
> just move the pg_free call to after the if-block.
I have fixed this with the attached, applied patch. I thought malloc(0)
would return null, but our src/common pg_malloc() has:
/* Avoid unportable behavior of malloc(0) */
if (size == 0)
size = 1;
so some memory is allocated, and has to be freed. I looked at avoiding
the call to gen_db_file_maps() for old_db->rel_arr.nrels == 0, but there
are checks in there comparing the old/new relation counts, so it can't
be skipped. I also removed the unnecessary memory initialization.
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachment | Content-Type | Size |
---|---|---|
diff | text/plain | 1.2 KB |
From: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
---|---|
To: | michael(dot)paquier(at)gmail(dot)com |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fixing memory leak in pg_upgrade |
Date: | 2015-01-10 02:19:44 |
Message-ID: | 20150110.111944.563424590546355319.t-ishii@sraoss.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
> On Fri, Jan 9, 2015 at 9:23 PM, Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
>> This is because gen_db_file_maps() allocates memory even if n_maps == 0.
> Purely cosmetic: the initialization n_maps = 0 before the call of
> gen_db_file_maps is unnecessary ;)
Of course. n_maps is written by calling gen_db_file_maps() anyway. I
was talking about the case after calling gen_db_file_maps().
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Tatsuo Ishii <ishii(at)postgresql(dot)org>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fixing memory leak in pg_upgrade |
Date: | 2015-01-10 12:16:46 |
Message-ID: | CAB7nPqQ3eSDT9j9TMwPEbageyZk2xKReNibxx7fZrMrmx2dhLw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Sat, Jan 10, 2015 at 2:27 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> so some memory is allocated, and has to be freed. I looked at avoiding
> the call to gen_db_file_maps() for old_db->rel_arr.nrels == 0, but there
> are checks in there comparing the old/new relation counts, so it can't
> be skipped. I also removed the unnecessary memory initialization.
Patch is fine IMO for its purpose.
--
Michael