From 726854951481af9f7210e5b9942abc8a4d9b55dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:53:36 +0000 Subject: [PATCH 1/4] Initial plan From 894577a6db52a7c4813f59dd994419b2b7d05f44 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Oct 2026 02:10:26 +0000 Subject: [PATCH 2/4] Fix exception propagation and rchk PROTECT stack depth in C++ Co-authored-by: dipterix <8163576+dipterix@users.noreply.github.com> --- src/core.cpp | 41 ++++++++++++++++++++--------------------- src/load.cpp | 4 ++-- src/map.cpp | 26 ++++++++++++++++---------- src/mapreduce.cpp | 12 ++++++------ 4 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/core.cpp b/src/core.cpp index 82b67e3..03f587d 100644 --- a/src/core.cpp +++ b/src/core.cpp @@ -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; } @@ -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(el), 1, dl, strict)); - n_protected++; + el = realToInt64(as(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(el), 1, dl, strict)); - n_protected++; + el = realToInt64(as(el), 1, dl, strict); } SET_VECTOR_ELT(sliceIdx, idx_size, el); } @@ -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(wrap(true))); @@ -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(wrap("integer64"))); lastptr2 = ((int64_t*) REAL(last_dim)) + l1; tmpptr = (int64_t*) REAL(tmp); @@ -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() @@ -520,6 +516,10 @@ List schedule(const SEXP listOrEnv, idx2len_cum += Rf_xlength(idx2tmp); idx2lens[part_size] = idx2len_cum; part_size++; + // `tmp` is kept alive by `sliceIdx2` and `idx2tmp` by `idx2s`, + // 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)){ @@ -529,7 +529,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(wrap("integer64"))); lastptr2 = ((int64_t*) REAL(last_dim)) + l1; tmpptr = (int64_t*) REAL(tmp); @@ -545,14 +544,14 @@ 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 `sliceIdx2`, `idx2tmp` by `idx2s`. + UNPROTECT(2); // tmp, idx2tmp } if( part_size > 0 ){ part_size--; diff --git a/src/load.cpp b/src/load.cpp index 81f8891..8b5bb53 100644 --- a/src/load.cpp +++ b/src/load.cpp @@ -243,11 +243,11 @@ SEXP FARR_subset_sequential( } } 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_subset_sequential`."); } diff --git a/src/map.cpp b/src/map.cpp index 3e9b558..7abce42 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -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; @@ -211,16 +214,16 @@ SEXP FARR_buffer_map( // 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()); + 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); } @@ -282,9 +285,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; @@ -320,19 +326,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); } diff --git a/src/mapreduce.cpp b/src/mapreduce.cpp index efcc383..439a2d8 100644 --- a/src/mapreduce.cpp +++ b/src/mapreduce.cpp @@ -63,9 +63,9 @@ SEXP each_partition_template( UNPROTECT( 1 ); // item } } 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 `each_partition_template`."); } @@ -208,11 +208,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`."); } @@ -232,9 +232,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`."); } From 9ee7cf0d1a7e063eca1046637c5e3a6388565103 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 11 Oct 2026 02:28:38 +0000 Subject: [PATCH 3/4] Clarify PROTECT protection-source comments in schedule() Co-authored-by: dipterix <8163576+dipterix@users.noreply.github.com> --- src/core.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core.cpp b/src/core.cpp index 03f587d..2955308 100644 --- a/src/core.cpp +++ b/src/core.cpp @@ -516,9 +516,11 @@ List schedule(const SEXP listOrEnv, idx2len_cum += Rf_xlength(idx2tmp); idx2lens[part_size] = idx2len_cum; part_size++; - // `tmp` is kept alive by `sliceIdx2` and `idx2tmp` by `idx2s`, - // so the per-iteration protections can be released here to keep - // the protection stack depth bounded. + // `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; @@ -550,7 +552,8 @@ List schedule(const SEXP listOrEnv, idx2len_cum += Rf_xlength(idx2tmp); idx2lens[part_size] = idx2len_cum; part_size++; - // `tmp` kept alive by `sliceIdx2`, `idx2tmp` by `idx2s`. + // `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 ){ From 8a5074220a0ac5314b5bd53fe968b8ee47895e6e Mon Sep 17 00:00:00 2001 From: dipterix Date: Sun, 11 Oct 2026 00:40:28 -0400 Subject: [PATCH 4/4] Test mapreduce error propagation; warn against std::rethrow_exception - Note at each file's first `throw;` why `std::rethrow_exception` must not come back: a package built with libc++ can run in an R that loaded libstdc++ (via ICU), and exception_ptr would mix the two C++ runtimes - Add a regression test that errors (and custom conditions) raised in mapreduce's map and reduce functions reach the caller unchanged - NEWS entry Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 1 + src/load.cpp | 3 +++ src/map.cpp | 4 +++- src/mapreduce.cpp | 3 +++ tests/testthat/test-map.R | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index a67b059..6637d5c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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 diff --git a/src/load.cpp b/src/load.cpp index 8b5bb53..35e1f92 100644 --- a/src/load.cpp +++ b/src/load.cpp @@ -243,6 +243,9 @@ SEXP FARR_subset_sequential( } } catch (const Rcpp::LongjumpException& e) { + // 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 diff --git a/src/map.cpp b/src/map.cpp index 7abce42..81c8515 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -213,7 +213,9 @@ 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 + // 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); diff --git a/src/mapreduce.cpp b/src/mapreduce.cpp index 439a2d8..11cb28d 100644 --- a/src/mapreduce.cpp +++ b/src/mapreduce.cpp @@ -63,6 +63,9 @@ SEXP each_partition_template( UNPROTECT( 1 ); // item } } catch (const Rcpp::LongjumpException& e) { + // 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) { throw; diff --git a/tests/testthat/test-map.R b/tests/testthat/test-map.R index 583741b..1a72c17 100644 --- a/tests/testthat/test-map.R +++ b/tests/testthat/test-map.R @@ -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) +})