Skip to content

Avoid potential protection stack overflow in bboxCalcR_c - #160

Merged
edzer merged 1 commit into
edzer:mainfrom
anashen:fix/add-unprotect-bbox-calc
Oct 11, 2026
Merged

edzer merged 1 commit into
edzer:mainfrom
anashen:fix/add-unprotect-bbox-calc

Conversation

@anashen

@anashen anashen commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Hello,

Thank you for your work on this package. I understand that it is in the process of being deprecated / is in maintenance mode; I'm hoping this PR can be considered as part of a patch update. It includes a fix for regression in behavior that seems to have been introduced in the latest version on CRAN, v2.2-3.

I was made aware of this issue via satijalab/seurat#10527.

For context: we currently use the SpatialPolygons class from your package to handle certain types of single-cell spatial transcriptomics data (and also hope to transition fully to sf soon). Recently, users with sp version v2.2-3 have reported Error: protect(): protection stack overflow on object-creation functions that use the SpatialPolygons constructor.

I looked into this by going through the changes introduced in v2.2-3, and I think the culprit is the bboxCalcR_c function. There are calls to PROTECT each polygon during the loop to go through all vertex coordinates to calculate the bounding box...

sp/src/sp_xports.c

Lines 468 to 479 in a4624aa

for (j=0; j<npl; j++) {
crds = PROTECT(GET_SLOT(VECTOR_ELT(Pl, j), Rf_install("coords"))); pc++;
n = INTEGER_POINTER(Rf_getAttrib(crds, R_DimSymbol))[0];
for (k=0; k<n; k++) {
x = NUMERIC_POINTER(crds)[k];
y = NUMERIC_POINTER(crds)[k+n];
if (x > UX) UX = x;
if (y > UY) UY = y;
if (x < LX) LX = x;
if (y < LY) LY = y;
}
}
...but when the number of polygons exceeds the default protection stack size, it can produce a protection stack overflow error.

This PR proposes to simply add a call to UNPROTECT crds, and decrement pc, after accessing the coordinates of all vertices for the given polygon.

Example

I've included a reprex of the error below. It runs successfully loading a checkout of this branch.

library(sp)

packageVersion("sp")
#> [1] '2.2.3'

# small triangle
polygon <- Polygons(
  list(Polygon(rbind(c(0, 0), c(1, 0), c(0, 1), c(0, 0)))),
  ID = "template"
)

# "large" object, 55k polygons
polygons <- lapply(seq_len(55000L), function(i) {
  slot(polygon, "ID") <- paste0("cell", i)
  polygon
})

# object creation succeeds for a smaller number of polygons
small <- SpatialPolygons(polygons[seq_len(40000L)])

# large input fails on object creation
large <- SpatialPolygons(polygons)
#> Error:
#> ! protect(): protection stack overflow

sessioninfo::session_info(pkgs = "attached")
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.5.2 (2025-10-31)
#>  os       Ubuntu 20.04.6 LTS
#>  system   x86_64, linux-gnu
#>  ui       X11
#>  language (EN)
#>  collate  en_US.UTF-8
#>  ctype    en_US.UTF-8
#>  tz       America/New_York
#>  date     2026-10-09
#>  pandoc   3.1.1 @ /opt/quarto/bin/tools/ (via rmarkdown)
#>  quarto   1.3.353 @ /usr/local/bin/quarto
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package * version date (UTC) lib source
#>  sp      * 2.2-3   2026-07-19 [1] CRAN (R 4.5.2)
#> 
#>  [1] /home/shenoya/R/x86_64-pc-linux-gnu-library/4.5
#>  [2] /usr/local/lib/R/site-library
#>  [3] /usr/lib/R/site-library
#>  [4] /usr/lib/R/library
#>  * ── Packages attached to the search path.
#> 
#> ──────────────────────────────────────────────────────────────────────────────

Created on 2026-10-09 with reprex v2.1.1

Tests

With this edit, all package tests pass (run locally under Ubuntu 20.04.6 LTS).

@edzer
edzer merged commit 81615ce into edzer:main Oct 11, 2026
6 checks passed
@edzer

edzer commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Nice catch - thanks!

edzer added a commit that referenced this pull request Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants