On 2015-07-23 22:50:12 +0900, Michael Paquier wrote:
>
> While running valgring on pg_rewind, I have noticed that each new call
> to XLogReadRecord leaks XLogReaderState->main_data and
> XLogReaderState->blocks[block_id].data because each one of them is
> palloc'd to store respectively the main data of the record and the
> data attached to each block. I think that the most correct fix to
> prevent the leak is to free those pointers when calling ResetDecoder()
> which is used to reset a reader state between two records.
I don't think that'd be a good fix. Isn't the idea here that we only
allocate new buffers when the previous one is too small? Your fix will
greatly increase the memory management overhead.
> ==46805== 154 bytes in 1 blocks are definitely lost in loss record 103 of 167
> ==46805== at 0x6B1B: malloc (in
> /usr/local/Cellar/valgrind/3.10.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
> ==46805== by 0x10000DB2D: pg_malloc_internal (fe_memutils.c:30)
> ==46805== by 0x10000DE79: palloc (fe_memutils.c:118)
> ==46805== by 0x1000052A0: DecodeXLogRecord (xlogreader.c:1203)
> ==46805== by 0x100003AA9: XLogReadRecord (xlogreader.c:461)
> ==46805== by 0x1000022F0: extractPageMap (parsexlog.c:78)
> ==46805== by 0x10000188C: main (pg_rewind.c:280)
Uh. This is the leak?
if (state->main_data_len > 0)
{
if (!state->main_data || state->main_data_len > state->main_data_bufsz)
{
if (state->main_data)
pfree(state->main_data);
state->main_data_bufsz = state->main_data_len;
state->main_data = palloc(state->main_data_bufsz); <--- here
}
memcpy(state->main_data, ptr, state->main_data_len);
ptr += state->main_data_len;
}
Where/When are we leaking memory here? The previously used memory is
freed before we allocate a larger buffer.
Greetings,
Andres Freund