Fix DBR::Misc::DBI::Compat dying on read-only row buffers#18
Merged
Conversation
DBD::mysql returns a reused internal row arrayref whose AV is flagged
SvREADONLY, so `@$row = map { ... } @$row` in fetchrow_arrayref and
fetchall_arrayref died with "Modification of a read-only value
attempted at .../Compat.pm line 19".
Build a fresh arrayref in both paths instead of reassigning in place.
Hash-slice paths are unchanged: hash slot assignment replaces the SV
and is safe regardless of the source AV's flags.
Adds rt_rop9213.t — a standalone regression test that mocks DBI::st
to hand back SvREADONLY row AVs; it reproduces the production error
on master and passes with the fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
lib/DBR/Misc/DBI/Compat.pm:fetchrow_arrayrefand the array-slice path offetchall_arrayrefnow return a freshly allocated arrayref instead of reassigning into@$row. Hash-slice paths are unchanged — hash slot assignment replaces the SV and is safe regardless of source AV flags.t/rt_rop9213.t(new): standalone regression test that mocksDBI::stto hand back row arrayrefs whose AV is flaggedSvREADONLY, reproducing the production error on master and passing with the fix.Background
After #17 merged, services using the new shim started dying with:
Line 19 was
@$row = map { defined $_ ? "$_" : $_ } @$row;.DBD::mysql'sfetchrow_arrayrefreturns a reused internal row buffer whose AV itself is flaggedSvREADONLY(not the individual scalars — verified empirically:Internals::SvREADONLY(@arr, 1)triggers the same croak on@$arr = ..., while flagging the elements does not). Building a fresh arrayref sidesteps the issue entirely.The same pattern existed at line 48 in
fetchall_arrayref's array-slice branch — fixed the same way.Test plan
prove -l t/rt_rop9213.tpasses with fix, fails on master with the exact production errorJira ticket
https://gudtech.atlassian.net/browse/ROP-9213
🤖 Generated with Claude Code