Skip to content

Backport #8845: Fix CDC decoder crash from translated tuples outliving the publish call (release-13.2) - #8914

Merged
ibrahim halatci (ihalatci) merged 2 commits into
bp-8465-release-13.2from
bp-8845-release-13.2
Oct 21, 2026
Merged

ibrahim halatci (ihalatci) merged 2 commits into
bp-8465-release-13.2from
bp-8845-release-13.2

Conversation

@ihalatci

Copy link
Copy Markdown
Contributor

Backports #8845 (commit 211afb1) to release-13.2.

Description

Scope the tuple substitution in \TranslateAndPublishRelationForCDC\ strictly to the publish call:

  • Remembers original tuple pointers before translating.
  • After \ouputPluginChangeCB\ returns, frees Citus-allocated translated tuples and restores the original pointers, preventing walsender segmentation faults in \ReorderBufferCleanupTXN.

Validation

Tested locally on PostgreSQL 17 with build, install, and CDC regression tests.

@ihalatci
ibrahim halatci (ihalatci) changed the base branch from release-13.2 to bp-8465-release-13.2 October 14, 2026 07:35
@ihalatci
ibrahim halatci (ihalatci) added this pull request to stack #8928 October 14, 2026 07:36
…ll (#8845)

## The crash

`cdc_decoder.c` segfaults the walsender. Proven stack from a local core
dump:

```text
SlabFree
ReorderBufferFreeChange
ReorderBufferCleanupTXN
ReorderBufferProcessTXN
xact_decode
LogicalDecodingProcessRecord
XLogSendLogical
WalSndLoop
```

```text
LOG:  client backend (PID ...) was terminated by signal 11: Segmentation fault
DETAIL:  Failed process was running: START_REPLICATION SLOT "cdc_replication_slot" LOGICAL 0/0
```

Core inspection pinned the faulting free to the field at struct offset
`0x38` of a
`REORDER_BUFFER_CHANGE_UPDATE` change — confirmed via DWARF to be
`data.tp.newtuple` —
and showed the pointer is the Citus-translated tuple, not PostgreSQL's
own. Its chunk
header selects allocator ID 5, from which `SlabFree` derives a bogus
context pointer
(`0x1000000`) and faults.

## Root cause: ownership / lifetime mismatch

- `GetTupleForTargetSchemaForCdc()` builds a replacement tuple with
`heap_form_tuple()`
  in the *current* memory context.
- `TranslateChangesIfSchemaChanged()` assigns it to
`change->data.tp.newtuple` (and to
`oldtuple` for `UPDATE` with `REPLICA IDENTITY FULL`, and for `DELETE`),
and never
  restores the original pointers.
- `TranslateAndPublishRelationForCDC()` called `ouputPluginChangeCB()`
and returned with
  both fields still pointing at Citus-allocated memory.
- PostgreSQL then frees both fields in `ReorderBufferFreeChange()`
during
`ReorderBufferCleanupTXN()`, expecting memory allocated from the reorder
buffer's own
  tuple context. It frees a foreign chunk and faults.

This is **not** a concurrency race and **not** a regression from any
recent PR. It is a
pre-existing latent bug that surfaces intermittently because it depends
on memory reuse,
which is why it has looked like a flaky test rather than a crash.

## The fix

Scope the tuple substitution strictly to the publish call, so PostgreSQL
only ever sees —
and therefore only ever frees — pointers it allocated itself.

In `TranslateAndPublishRelationForCDC()`:

1. Remember `change->data.tp.newtuple` / `oldtuple` before translating.
2. After `ouputPluginChangeCB()` returns, for each field: if it no
longer holds the
original pointer, `heap_freetuple()` the tuple Citus allocated and
restore the original.

Using "differs from the original" as the test makes restoration
automatically correct on
every translated path — `INSERT` (newtuple only), `UPDATE` with and
without a non-NULL
`oldtuple`, and `DELETE` (oldtuple only) — and a strict no-op when
`HasSchemaChanged()`
returned false and nothing was replaced.

Deliberately minimal: 29 added lines in one function, roughly half of
them comments.
No signature changes, no change to `GetTupleForTargetSchemaForCdc()`'s
translation logic,
no new memory contexts, no change to the pgoutput callback contract, and
no
version `#ifdef` — the minimum supported version is PG 17 and
`data.tp.newtuple` is a
plain `HeapTuple` on 17/18/19, which is why the surrounding code already
assigns it
unguarded.

## Testing

`src/test/cdc/t/006_cdc_schema_change_and_move.pl` exercises exactly
this path: it
combines `DROP COLUMN`, a `force_logical` shard move, and a subsequent
`UPDATE`, which is
what drives translation and therefore the bad free. CI will validate it.

Locally, the extension was built against PostgreSQL 18.6: `citus.so`,
`citus_columnar.so`,
`citus_pgoutput.so`, and `citus_wal2json.so` all compile and link
cleanly, with no new
compiler warnings under the project's `-Wall -Wextra` flags.
`cdc_decoder.c` is clean under
`uncrustify` with the repo's `citus-style.cfg`.

## Follow-up (not in this PR)

`.github/actions/save_logs_and_results` does not upload
`src/test/cdc/log/`, so when the
walsender crashes, CI shows only a `poll_query_until` catch-up timeout
with "connection
refused" and the real server log is lost. Worth fixing separately to
make future CDC
crashes diagnosable — intentionally left out here to keep this change to
the fix itself.

Fixes #8844

---------

Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 211afb1)
Copilot-Session: d89d1f43-18ea-4405-bd1b-5c3129e391a9
On PostgreSQL 15 and 16, ReorderBufferChange data.tp.newtuple and oldtuple
are ReorderBufferTupleBuf pointers rather than HeapTuples, and their pointers
are not replaced during translation. Guard originalNewTuple and originalOldTuple
restoration and freeing with PG_VERSION_NUM >= PG_VERSION_17.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d89d1f43-18ea-4405-bd1b-5c3129e391a9
@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (bp-8465-release-13.2@3de1204). Learn more about missing BASE report.

Additional details and impacted files
@@                   Coverage Diff                   @@
##             bp-8465-release-13.2    #8914   +/-   ##
=======================================================
  Coverage                        ?   29.15%           
=======================================================
  Files                           ?      289           
  Lines                           ?    64354           
  Branches                        ?     8067           
=======================================================
  Hits                            ?    18764           
  Misses                          ?    43397           
  Partials                        ?     2193           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ihalatci
ibrahim halatci (ihalatci) merged commit 702dce4 into release-13.2 Oct 21, 2026
188 of 241 checks passed
@ihalatci
ibrahim halatci (ihalatci) deleted the bp-8845-release-13.2 branch October 21, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant