Re: Memory leak in XLOG reader facility when decoding records (caused by WAL refactoring)

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