Skip to content

Fix failing CI: exception propagation across toolchains and rchk PROTECT stack depth #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 4 commits into from
Closed

Fix failing CI: exception propagation across toolchains and rchk PROTECT stack depth #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7268549
Initial plan
Copilot Oct 11, 2026
894577a
Fix exception propagation and rchk PROTECT stack depth in C++
Copilot Oct 11, 2026
9ee7cf0
Clarify PROTECT protection-source comments in schedule()
Copilot Oct 11, 2026
8a50742
Test mapreduce error propagation; warn against std::rethrow_exception
dipterix Oct 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Partition locks never make a write fail when the lock itself cannot be taken: any error while locking (for example a stale network file handle or too many open files, not only file systems without lock support) now warns once and the write proceeds, and an invalid `filearray.lock.timeout` option or `FILEARRAY_LOCK_TIMEOUT` value warns once and is ignored instead of stopping every write
* Fixed `mapreduce` (and so `sum`, `max`, `min`, `range` and `fwhich`) reading only the first slice of every partition after the first when `partition_size` is greater than 1, which gave wrong results and wrong `fwhich` indices
* `fmap2` reports the error raised by the mapping function instead of "Unknown error."
* Errors and interrupts raised in the functions passed to `fmap`, `fmap2` and `mapreduce` now reach the caller unchanged on Linux builds that compile packages with `libc++` while R itself loads `libstdc++` (for example through ICU). Before, they arrived as "c++ exception (unknown reason)" and could corrupt memory and crash the R session

# filearray 0.2.3

Expand Down
44 changes: 23 additions & 21 deletions src/core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,7 @@ SEXP locationList(const SEXP listOrEnv, const NumericVector& dim, const int stri
UNPROTECT(n_protected); // sliceIdx, dots
stop("Incorrect subscript dimensions, required: 0, ndim.");
}
SET_VECTOR_ELT(sliceIdx, idx_size, PROTECT(CAR(dots)));
n_protected++;
SET_VECTOR_ELT(sliceIdx, idx_size, CAR(dots));
}
break;
}
Expand Down Expand Up @@ -310,24 +309,24 @@ SEXP locationList(const SEXP listOrEnv, const NumericVector& dim, const int stri
// el = PROTECT(Rf_eval( PREXPR(el), PRENV( el )));
// From R 4.5.0, PREXPR and PRENV are deprecated
// Directly evaluate the promise instead
el = PROTECT(Rf_eval( el, listOrEnv ));
n_protected++;
//
// The evaluated promise `ev` is PROTECTed only until the
// coerced result is stored into `sliceIdx` (which then
// protects it), so the protection stack stays bounded.
SEXP ev = PROTECT(Rf_eval( el, listOrEnv ));

if( el == R_NilValue ){
el = PROTECT(Rf_allocVector(REALSXP, 0));
n_protected++;
if( ev == R_NilValue ){
el = Rf_allocVector(REALSXP, 0);
} else {
el = PROTECT(realToInt64(as<NumericVector>(el), 1, dl, strict));
n_protected++;
el = realToInt64(as<NumericVector>(ev), 1, dl, strict);
}
SET_VECTOR_ELT(sliceIdx, idx_size, el);
UNPROTECT(1); // ev
} else {
if( el == R_MissingArg || el == R_NilValue ){
el = PROTECT(seq_len_int64( dl ));
n_protected++;
el = seq_len_int64( dl );
} else {
el = PROTECT(realToInt64(as<NumericVector>(el), 1, dl, strict));
n_protected++;
el = realToInt64(as<NumericVector>(el), 1, dl, strict);
}
SET_VECTOR_ELT(sliceIdx, idx_size, el);
}
Expand All @@ -336,9 +335,8 @@ SEXP locationList(const SEXP listOrEnv, const NumericVector& dim, const int stri
sliceIdx = PROTECT(Rf_allocVector(VECSXP, ndims));
n_protected++;
for(idx_size = 0; idx_size < ndims; idx_size++ ){
el = PROTECT(seq_len_int64(dim[idx_size]));
SET_VECTOR_ELT(sliceIdx, idx_size, el);
n_protected++;
// `sliceIdx` protects each element once stored.
SET_VECTOR_ELT(sliceIdx, idx_size, seq_len_int64(dim[idx_size]));
}
}
Rf_setAttrib(sliceIdx, Rf_install("_asis_"), Shield<SEXP>(wrap(true)));
Expand Down Expand Up @@ -495,7 +493,6 @@ List schedule(const SEXP listOrEnv,
// change partition, flush!
if( l2 > l1 ){
SEXP tmp = PROTECT(Rf_allocVector(REALSXP, l2 - l1));
n_protected++;
Rf_setAttrib(tmp, R_ClassSymbol, Shield<SEXP>(wrap("integer64")));
lastptr2 = ((int64_t*) REAL(last_dim)) + l1;
tmpptr = (int64_t*) REAL(tmp);
Expand All @@ -511,7 +508,6 @@ List schedule(const SEXP listOrEnv,
dim2[ndim2 - 1] = cum_part_sizes[part] - part_start;

SEXP idx2tmp = PROTECT(loc2idx(sliceIdx2, dim2));
n_protected++;
idx2s.push_back(idx2tmp);

// partitions.insert()
Expand All @@ -520,6 +516,12 @@ List schedule(const SEXP listOrEnv,
idx2len_cum += Rf_xlength(idx2tmp);
idx2lens[part_size] = idx2len_cum;
part_size++;
// `tmp` is kept alive by the PROTECTed `sliceIdx2` and
// `idx2tmp` by the Rcpp `List idx2s` (whose backing SEXP is
// R-preserved after push_back), so the per-iteration
// protections can be released here to keep the protection
// stack depth bounded.
UNPROTECT(2); // tmp, idx2tmp
}
part = 0;
while(cum_part_sizes[part] < *lastptr || (part > 0 && cum_part_sizes[part-1] >= *lastptr)){
Expand All @@ -529,7 +531,6 @@ List schedule(const SEXP listOrEnv,
}
if( l2 > l1 ){
SEXP tmp = PROTECT(Rf_allocVector(REALSXP, l2 - l1));
n_protected++;
Rf_setAttrib(tmp, R_ClassSymbol, Shield<SEXP>(wrap("integer64")));
lastptr2 = ((int64_t*) REAL(last_dim)) + l1;
tmpptr = (int64_t*) REAL(tmp);
Expand All @@ -545,14 +546,15 @@ List schedule(const SEXP listOrEnv,
dim2[ndim2 - 1] = cum_part_sizes[part] - part_start;

SEXP idx2tmp = PROTECT(loc2idx(sliceIdx2, dim2));
n_protected++;
idx2s.push_back(idx2tmp);
// partitions.push_back(part);
partitions[part_size] = part;
idx2len_cum += Rf_xlength(idx2tmp);
idx2lens[part_size] = idx2len_cum;
part_size++;

// `tmp` kept alive by the PROTECTed `sliceIdx2`, `idx2tmp` by the
// Rcpp `List idx2s` (R-preserved after push_back).
UNPROTECT(2); // tmp, idx2tmp
}
if( part_size > 0 ){
part_size--;
Expand Down
7 changes: 5 additions & 2 deletions src/load.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,14 @@ SEXP FARR_subset_sequential(
}

} catch (const Rcpp::LongjumpException& e) {
std::rethrow_exception(std::current_exception());
// Use `throw;`, never `std::rethrow_exception`: a package
// built with libc++ can run in an R that loaded libstdc++,
// and `exception_ptr` would mix the two C++ runtimes
throw;
} catch (const boost::interprocess::interprocess_exception& e) {
// unable to find the file, skip
} catch (const std::exception& e) {
std::rethrow_exception(std::current_exception());
throw;
} catch (...) {
throw std::runtime_error("filearray C++: Caught an unknown exception in `FARR_subset_sequential`.");
}
Expand Down
30 changes: 19 additions & 11 deletions src/map.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,12 @@ SEXP FARR_buffer_map(


// allocate buffers
// `argbuffers` is PROTECTed once; every element stored via SET_VECTOR_ELT
// is protected by the container, so the elements are not PROTECTed
// individually (this keeps the protection stack depth bounded).
SEXP argbuffers = PROTECT(Rf_allocVector(VECSXP, narrays));
for(int ii = 0; ii < narrays; ii++){
SET_VECTOR_ELT(argbuffers, ii, PROTECT(Rf_allocVector(memory_buffer_types[ii], buffer_nelems[ii])));
SET_VECTOR_ELT(argbuffers, ii, Rf_allocVector(memory_buffer_types[ii], buffer_nelems[ii]));
}

int64_t current_pos = 0;
Expand Down Expand Up @@ -210,17 +213,19 @@ SEXP FARR_buffer_map(
} catch (const Rcpp::LongjumpException& e) {
// An R error or interrupt in `map` (including a partition lock
// time-out): let R continue the jump, which also resets the
// protection stack
std::rethrow_exception(std::current_exception());
// protection stack. Use `throw;`, never `std::rethrow_exception`:
// a package built with libc++ can run in an R that loaded
// libstdc++, and `exception_ptr` would mix the two C++ runtimes
throw;
} catch(std::exception &ex){
UNPROTECT(2 + narrays);
UNPROTECT(2);
forward_exception_to_r(ex);
} catch(...){
// UNPROTECT(2 + narrays);
// UNPROTECT(2);
Rcpp::warning("C++ `FARR_buffer_map`: cannot finish map");
}

UNPROTECT(2 + narrays);
UNPROTECT(2);

return(R_NilValue);
}
Expand Down Expand Up @@ -282,9 +287,12 @@ SEXP FARR_buffer_map2(


// allocate buffers
// `argbuffers` is PROTECTed once; every element stored via SET_VECTOR_ELT
// is protected by the container, so the elements are not PROTECTed
// individually (this keeps the protection stack depth bounded).
SEXP argbuffers = PROTECT(Rf_allocVector(VECSXP, narrays));
for(int ii = 0; ii < narrays; ii++){
SET_VECTOR_ELT(argbuffers, ii, PROTECT(Rf_allocVector(memory_buffer_types[ii], buffer_nelems[ii])));
SET_VECTOR_ELT(argbuffers, ii, Rf_allocVector(memory_buffer_types[ii], buffer_nelems[ii]));
}

int64_t current_pos = 0;
Expand Down Expand Up @@ -320,19 +328,19 @@ SEXP FARR_buffer_map2(
} catch (const Rcpp::LongjumpException& e) {
// An R error or interrupt in `map`: let R continue the jump with
// the original condition, which also resets the protection stack
std::rethrow_exception(std::current_exception());
throw;
} catch(std::exception &ex){
UNPROTECT(2 + narrays);
UNPROTECT(2);
forward_exception_to_r(ex);
} catch(...){
UNPROTECT(2 + narrays);
UNPROTECT(2);
stop("Unknown error.");
}


}

UNPROTECT(2 + narrays);
UNPROTECT(2);

return(ret);
}
Expand Down
15 changes: 9 additions & 6 deletions src/mapreduce.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ SEXP each_partition_template(
UNPROTECT( 1 ); // item
}
} catch (const Rcpp::LongjumpException& e) {
std::rethrow_exception(std::current_exception());
// Use `throw;`, never `std::rethrow_exception`: a package
// built with libc++ can run in an R that loaded libstdc++,
// and `exception_ptr` would mix the two C++ runtimes
throw;
} catch (const std::exception& e) {
std::rethrow_exception(std::current_exception());
throw;
} catch (...) {
throw std::runtime_error("filearray C++: Caught an unknown exception in `each_partition_template`.");
}
Expand Down Expand Up @@ -208,11 +211,11 @@ SEXP FARR_buffer_mapreduce(
}
}
} catch (const Rcpp::LongjumpException& e) {
std::rethrow_exception(std::current_exception());
throw;
} catch (const boost::interprocess::interprocess_exception& e) {
// unable to find the file, skip
} catch (const std::exception& e) {
std::rethrow_exception(std::current_exception());
throw;
} catch (...) {
throw std::runtime_error("filearray C++: Caught an unknown exception in `FARR_buffer_mapreduce`.");
}
Expand All @@ -232,9 +235,9 @@ SEXP FARR_buffer_mapreduce(
return(re);

} catch (const Rcpp::LongjumpException& e) {
std::rethrow_exception(std::current_exception());
throw;
} catch (const std::exception& e) {
std::rethrow_exception(std::current_exception());
throw;
} catch (...) {
throw std::runtime_error("filearray C++: Caught an unknown exception in `FARR_buffer_mapreduce`.");
}
Expand Down
32 changes: 32 additions & 0 deletions tests/testthat/test-map.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,35 @@ test_that("fmap2 passes on errors raised by the mapping function", {
sum(input[[1]])
}, .buffer_count = 2), c(21, 57))
})

test_that("mapreduce passes on errors raised by map and reduce", {
x <- filearray_create(temp_path(check = TRUE), c(2, 3, 2), partition_size = 1)
on.exit(x$delete(force = TRUE), add = TRUE)
x[] <- 1:12

expect_error(mapreduce(x, function(data) {
stop("boom from mapreduce map")
}), "boom from mapreduce map")

expect_error(mapreduce(x, function(data, size) {
sum(data[seq_len(size)])
}, reduce = function(mapped) {
stop("boom from mapreduce reduce")
}), "boom from mapreduce reduce")

# the condition itself reaches the caller, not only its message
cond <- structure(
class = c("mapreduce_test_error", "error", "condition"),
list(message = "custom condition", call = NULL)
)
expect_error(mapreduce(x, function(data) {
stop(cond)
}), class = "mapreduce_test_error")

# and mapreduce still works afterwards
expect_equal(mapreduce(x, function(data, size) {
sum(data[seq_len(size)])
}, reduce = function(mapped) {
sum(unlist(mapped))
}), 78)
})
Loading