protect fresh proxy get() results in conversion and assignment operators - #1492
Open
kevinushey wants to merge 1 commit into
Open
protect fresh proxy get() results in conversion and assignment operators#1492kevinushey wants to merge 1 commit into
kevinushey wants to merge 1 commit into
Conversation
Proxy operator T() conversions and proxy-to-proxy assignments passed the raw result of get() into allocating calls (as<T>() coercions, set() implementations that allocate before capturing the value). When get() returns a freshly computed SEXP -- an active binding or active RefClass field, a field access evaluated via $, an attribute expanded by Rf_getAttrib() -- that object is referenced from nowhere the garbage collector can see, and a collection triggered inside the window frees it while still in use. Reproducible via gctorture() on an R built with structure checking; see #1491 for details and a reproducible example. Shield the get() result at each such site. Also fix Binding::operator=(const Binding&), whose self-assignment guard ('*this != rhs') failed to compile whenever the operator was actually instantiated; the new regression test is its first instantiation. Fixes #1491.
eddelbuettel
reviewed
Aug 3, 2026
| if( *this != rhs ) | ||
| set( rhs.get() ) ; | ||
| // NB: '*this != rhs' previously used here would not compile | ||
| // when this operator was instantiated |
Member
There was a problem hiding this comment.
Odd. Thanks for persisting here too.
eddelbuettel
approved these changes
Aug 3, 2026
eddelbuettel
left a comment
Member
There was a problem hiding this comment.
Thank you! The actual change is, as you said, somewhat mechanical.
It's good to get to these.
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
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.
Fixes #1491.
The proxy
operator T()conversions and the proxy-to-proxy assignment operators passed the raw result ofget()into allocating calls:as<T>()coercions on the conversion side, andset()implementations that allocate before capturing the value on the assignment side. Whenget()returns a freshly computed SEXP (an active binding or active RefClass field, a field access evaluated via$, an attribute expanded byRf_getAttrib()), that object is referenced from nowhere the GC can see, and a collection inside the window frees it while still in use. See #1491 for the analysis, including why standard builds of recent R rarely surface this (internal protection incoerceVector()since R 4.1.0, andR_UnwindProtect()accidentally keeping eval results reachable via the continuation token).Changes:
api/meat/proxy.h:Shieldtheget()result in all thirteenoperator T()conversions and inFieldProxy::operator=(const FieldProxy&).proxy/SlotProxy.h: same forconst_SlotProxy::operator T()(defined inline) andSlotProxy::operator=(const SlotProxy&).proxy/Binding.h,proxy/NamesProxy.h: same for the proxy-to-proxy assignments.Binding::operator=(const Binding&)additionally had a latent compile error: its self-assignment guard*this != rhsis ambiguous (the template conversion operators make it match unrelated!=overloads), so the operator failed to compile whenever it was actually instantiated. The guard now compares the environment and name directly; the new regression test is this operator's first instantiation.AttributeProxy::operator=(const AttributeProxy&)(itsset()shields immediately upon entry) andgeneric_name_proxy::operator T()invector/proxy.h(itsget()returns a parent-reachable element).Verification:
unprotected object (0x...) encountered (was ENVSXP)against current master and runs clean with this patch.FieldProxyandBindingcopies from actively-computed values undergctorture(); they pass on standard builds and guard the fix on protect-checking builds such as CRAN's ASan setup.Checklist
R CMD checkstill passes all tests (checked locally withRunAllRcppTests=yeson R 4.6.1;--no-manual --no-vignettes, so the only flagged items were two vignette-packaging warnings from the local--no-build-vignettesbuild)