Skip to content

Commit 6509a86

Browse files
committed
protect new cons cell in named StretchyList::push_back()
The named variant of push_back() allocated the new cons cell before constructing the Symbol for its tag. If the tag name was not yet interned, Rf_install() could allocate and trigger a garbage collection that reclaimed the still-unreachable cell. Construct the Symbol first, matching push_front(). Fixes #1489.
1 parent 0c684f0 commit 6509a86

4 files changed

Lines changed: 25 additions & 1 deletion

File tree

ChangeLog

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@
2222
* inst/include/Rcpp/proxy/NamesProxy.h: Idem
2323
* inst/include/Rcpp/proxy/SlotProxy.h: Idem
2424
* inst/tinytest/cpp/misc.cpp: Add regression tests
25+
26+
2026-08-02 Kevin Ushey <kevinushey@gmail.com>
27+
28+
* inst/include/Rcpp/api/meat/StretchyList.h: Protect the new cell
29+
from collection during Rf_install() in named push_back() (#1489)
30+
* inst/tinytest/cpp/misc.cpp: Add regression test
2531
* inst/tinytest/test_misc.R: Idem
2632

2733
2026-07-24 Dirk Eddelbuettel <edd@debian.org>

inst/include/Rcpp/api/meat/StretchyList.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ namespace Rcpp{
3535
template< typename T>
3636
StretchyList_Impl<StoragePolicy>& StretchyList_Impl<StoragePolicy>::push_back__impl( const T& obj, traits::true_type ){
3737
Shield<SEXP> s( wrap(obj.object) ) ;
38-
SEXP tmp = Rf_cons( s, R_NilValue );
3938
Symbol tag = obj.name ;
39+
SEXP tmp = Rf_cons( s, R_NilValue );
4040
SET_TAG(tmp, tag) ;
4141
SEXP self = Storage::get__() ;
4242
SETCDR( CAR(self), tmp) ;

inst/tinytest/cpp/misc.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,13 @@ StretchyList named_stretchy_list() {
173173
return out;
174174
}
175175

176+
// [[Rcpp::export]]
177+
StretchyList named_stretchy_list_dynamic(std::string name) {
178+
StretchyList out;
179+
out.push_back( Named(name, 42) );
180+
return out;
181+
}
182+
176183
// [[Rcpp::export]]
177184
void copy_field_gc(Reference a, Reference b) {
178185
a.field("x") = b.field("y");

inst/tinytest/test_misc.R

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,17 @@ expect_equal(stretchy_list(), pairlist( "foo", 1L, 3.2 ))
129129
# test.named_StretchyList <- function(){
130130
expect_equal(named_stretchy_list(), pairlist( a = "foo", b = 1L, c = 3.2 ))
131131

132+
# test.named_StretchyList_gc <- function(){
133+
## push_back() must keep the new cell protected across the Rf_install()
134+
## needed for a not-yet-interned tag name (#1489)
135+
name <- paste(sample(c(letters, LETTERS), 32, TRUE), collapse = "")
136+
gctorture(TRUE)
137+
result <- named_stretchy_list_dynamic(name)
138+
gctorture(FALSE)
139+
expected <- pairlist(42L)
140+
names(expected) <- name
141+
expect_equal(result, expected)
142+
132143
# test.FieldProxy.gc <- function(){
133144
## copying a field whose value is computed freshly on access must keep
134145
## that value protected across the assignment (#1491)

0 commit comments

Comments
 (0)