On Tue, Mar 7, 2023 at 2:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> I've found the following explanation for the failure: > 1) After the ExecGetUpdateNewTuple() call the newslot and the oldslot are > linked together (their slot->tts_values[1] in this case point to the same > memory address (inside the oldslot' buffer)). > 2) Previously, GetTupleForTrigger() could get a tuple with a new buffer, > so the oldslot would be the only user of the buffer at that moment. > (The newslot doesn't become an official user of the buffer.) > 3) Then, trigtuple = ExecFetchSlotHeapTuple(oldslot, ...) invokes > tts_buffer_heap_materialize() where the oldslot->buffer is released. > 4) Finally, newtuple = ExecFetchSlotHeapTuple(newslot, ...) invokes > tts_buffer_heap_materialize() where an incorrect access to memory > that became anonymous occurs, and that is detected by valgrind. > If not detected, different consequences are possible (in the asan case > it was memcpy with an incorrectly read extra large data_len). I've tried to materialize newslot before the oldslot materialization (in ExecFetchSlotHeapTuple(), where their common memory is released), and it looks like it fixes the issue.
Reproduced this issue on master with your queries. I looked into this issue and I agree with your analysis. I think this is exactly what happened.
I also agree that we should materialize the newslot before we fetch trigtuple from the oldslot which would materialize the oldslot and release all buffer pins. But I'm not too familiar with the arounding codes so need someone else to have a look.