Commit 4ddd8f5f authored by Tomas Vondra's avatar Tomas Vondra

Fix memory leak in TRUNCATE decoding

When decoding a TRUNCATE record, the relids array was being allocated in
the main ReorderBuffer memory context, but not released with the change
resulting in a memory leak.

The array was also ignored when serializing/deserializing the change,
assuming all the information is stored in the change itself.  So when
spilling the change to disk, we've only we have serialized only the
pointer to the relids array.  Thanks to never releasing the array,
the pointer however remained valid even after loading the change back
to memory, preventing an actual crash.

This fixes both the memory leak and (de)serialization.  The relids array
is still allocated in the main ReorderBuffer memory context (none of the
existing ones seems like a good match, and adding an extra context seems
like an overkill).  The allocation is wrapped in a new ReorderBuffer API
functions, to keep the details within reorderbuffer.c, just like the
other ReorderBufferGet methods do.

Author: Tomas Vondra
Discussion: https://www.postgresql.org/message-id/flat/66175a41-9342-2845-652f-1bd4c3ee50aa%402ndquadrant.com
Backpatch: 11, where decoding of TRUNCATE was introduced
parent caa0c6ce
...@@ -859,7 +859,8 @@ DecodeTruncate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) ...@@ -859,7 +859,8 @@ DecodeTruncate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
if (xlrec->flags & XLH_TRUNCATE_RESTART_SEQS) if (xlrec->flags & XLH_TRUNCATE_RESTART_SEQS)
change->data.truncate.restart_seqs = true; change->data.truncate.restart_seqs = true;
change->data.truncate.nrelids = xlrec->nrelids; change->data.truncate.nrelids = xlrec->nrelids;
change->data.truncate.relids = palloc(xlrec->nrelids * sizeof(Oid)); change->data.truncate.relids = ReorderBufferGetRelids(ctx->reorder,
xlrec->nrelids);
memcpy(change->data.truncate.relids, xlrec->relids, memcpy(change->data.truncate.relids, xlrec->relids,
xlrec->nrelids * sizeof(Oid)); xlrec->nrelids * sizeof(Oid));
ReorderBufferQueueChange(ctx->reorder, XLogRecGetXid(r), ReorderBufferQueueChange(ctx->reorder, XLogRecGetXid(r),
......
...@@ -409,10 +409,16 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change) ...@@ -409,10 +409,16 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change)
} }
break; break;
/* no data in addition to the struct itself */ /* no data in addition to the struct itself */
case REORDER_BUFFER_CHANGE_TRUNCATE:
if (change->data.truncate.relids != NULL)
{
ReorderBufferReturnRelids(rb, change->data.truncate.relids);
change->data.truncate.relids = NULL;
}
break;
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
case REORDER_BUFFER_CHANGE_TRUNCATE:
break; break;
} }
...@@ -450,6 +456,37 @@ ReorderBufferReturnTupleBuf(ReorderBuffer *rb, ReorderBufferTupleBuf *tuple) ...@@ -450,6 +456,37 @@ ReorderBufferReturnTupleBuf(ReorderBuffer *rb, ReorderBufferTupleBuf *tuple)
pfree(tuple); pfree(tuple);
} }
/*
* Get an array for relids of truncated relations.
*
* We use the global memory context (for the whole reorder buffer), because
* none of the existing ones seems like a good match (some are SLAB, so we
* can't use those, and tup_context is meant for tuple data, not relids). We
* could add yet another context, but it seems like an overkill - TRUNCATE is
* not particularly common operation, so it does not seem worth it.
*/
Oid *
ReorderBufferGetRelids(ReorderBuffer *rb, int nrelids)
{
Oid *relids;
Size alloc_len;
alloc_len = sizeof(Oid) * nrelids;
relids = (Oid *) MemoryContextAlloc(rb->context, alloc_len);
return relids;
}
/*
* Free an array of relids.
*/
void
ReorderBufferReturnRelids(ReorderBuffer *rb, Oid *relids)
{
pfree(relids);
}
/* /*
* Return the ReorderBufferTXN from the given buffer, specified by Xid. * Return the ReorderBufferTXN from the given buffer, specified by Xid.
* If create is true, and a transaction doesn't already exist, create it * If create is true, and a transaction doesn't already exist, create it
...@@ -2412,6 +2449,26 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn, ...@@ -2412,6 +2449,26 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
break; break;
} }
case REORDER_BUFFER_CHANGE_TRUNCATE: case REORDER_BUFFER_CHANGE_TRUNCATE:
{
Size size;
char *data;
/* account for the OIDs of truncated relations */
size = sizeof(Oid) * change->data.truncate.nrelids;
sz += size;
/* make sure we have enough space */
ReorderBufferSerializeReserve(rb, sz);
data = ((char *) rb->outbuf) + sizeof(ReorderBufferDiskChange);
/* might have been reallocated above */
ondisk = (ReorderBufferDiskChange *) rb->outbuf;
memcpy(data, change->data.truncate.relids, size);
data += size;
break;
}
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
...@@ -2695,6 +2752,16 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn, ...@@ -2695,6 +2752,16 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
} }
/* the base struct contains all the data, easy peasy */ /* the base struct contains all the data, easy peasy */
case REORDER_BUFFER_CHANGE_TRUNCATE: case REORDER_BUFFER_CHANGE_TRUNCATE:
{
Oid *relids;
relids = ReorderBufferGetRelids(rb,
change->data.truncate.nrelids);
memcpy(relids, data, change->data.truncate.nrelids * sizeof(Oid));
change->data.truncate.relids = relids;
break;
}
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM: case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM:
case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID: case REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID:
case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID: case REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID:
......
...@@ -402,6 +402,9 @@ void ReorderBufferReturnTupleBuf(ReorderBuffer *, ReorderBufferTupleBuf *tuple) ...@@ -402,6 +402,9 @@ void ReorderBufferReturnTupleBuf(ReorderBuffer *, ReorderBufferTupleBuf *tuple)
ReorderBufferChange *ReorderBufferGetChange(ReorderBuffer *); ReorderBufferChange *ReorderBufferGetChange(ReorderBuffer *);
void ReorderBufferReturnChange(ReorderBuffer *, ReorderBufferChange *); void ReorderBufferReturnChange(ReorderBuffer *, ReorderBufferChange *);
Oid * ReorderBufferGetRelids(ReorderBuffer *, int nrelids);
void ReorderBufferReturnRelids(ReorderBuffer *, Oid *relids);
void ReorderBufferQueueChange(ReorderBuffer *, TransactionId, XLogRecPtr lsn, ReorderBufferChange *); void ReorderBufferQueueChange(ReorderBuffer *, TransactionId, XLogRecPtr lsn, ReorderBufferChange *);
void ReorderBufferQueueMessage(ReorderBuffer *, TransactionId, Snapshot snapshot, XLogRecPtr lsn, void ReorderBufferQueueMessage(ReorderBuffer *, TransactionId, Snapshot snapshot, XLogRecPtr lsn,
bool transactional, const char *prefix, bool transactional, const char *prefix,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment