traversc / qs

Quick serialization of R objects
397 stars 19 forks source link

qsave() bad binding access #94

Closed MarekGierlinski closed 3 months ago

MarekGierlinski commented 4 months ago

I have come across an error involving cowplot::plot_grid() and qs::qsave(). I use targets with option format = "qs" and after recent updates some of my pipelines started to experience crashes. I tried using older version of qs and cowplot, but the error persisted, so I suspect it is happened after upgrading R to 4.4.0.

Here is a reproducible (I hope) example showing when the error happens. The code below creates two ggplot2 objects with nearly identical images. One is created with cowplot::grid_plot() with other with patchwork. The object created by patchwork is saved with no issues, but saving the object created with cowplot crashes with "bad binding access".

library(ggplot2)
library(cowplot)
library(patchwork)
#> 
#> Attaching package: 'patchwork'
#> The following object is masked from 'package:cowplot':
#> 
#>     align_plots
library(qs)
#> qs 0.26.1

make_plot1 <- function() {
  pl1 <- ggplot(mtcars, aes(x = hp, y = mpg)) + geom_point()
  pl2 <- ggplot(mtcars, aes(x = disp, y = drat)) + geom_point()
  pl1 + pl2
}

make_plot2 <- function() {
  pl1 <- ggplot(mtcars, aes(x = hp, y = mpg)) + geom_point()
  pl2 <- ggplot(mtcars, aes(x = disp, y = drat)) + geom_point()
  plot_grid(pl1, pl2)
}

obj1 <- make_plot1()
obj2 <- make_plot2()

qsave(obj1, "test1.qs")
qsave(obj2, "test2.qs")
#> Error in qsave(obj2, "test2.qs"): bad binding access

Here is my session info:

R version 4.4.0 (2024-04-24)
Platform: x86_64-apple-darwin20
Running under: macOS Sonoma 14.4.1

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib 
LAPACK: /Library/Frameworks/R.framework/Versions/4.4-x86_64/Resources/lib/libRlapack.dylib;  LAPACK version 3.12.0

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

time zone: Europe/London
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices datasets  utils     methods   base     

other attached packages:
[1] qs_0.26.1       patchwork_1.2.0 cowplot_1.1.3   ggplot2_3.5.1  

loaded via a namespace (and not attached):
 [1] vctrs_0.6.5         cli_3.6.2           rlang_1.1.3         renv_1.0.7          RcppParallel_5.1.7 
 [6] glue_1.7.0          labeling_0.4.3      colorspace_2.1-0    RApiSerialize_0.1.2 stringfish_0.16.0  
[11] scales_1.3.0        fansi_1.0.6         grid_4.4.0          munsell_0.5.1       tibble_3.2.1       
[16] lifecycle_1.0.4     compiler_4.4.0      Rcpp_1.0.12         pkgconfig_2.0.3     farver_2.1.1       
[21] R6_2.5.1            utf8_1.2.4          pillar_1.9.0        magrittr_2.0.3      tools_4.4.0        
[26] withr_3.0.0         gtable_0.3.5       
traversc commented 4 months ago

Thank you, I can reproduce. Looks like there is a change in R 4.4.

traversc commented 4 months ago

Could you try the latest commit? This was related to the change in how R handled promises. The solution was to force evaluation of promises before serialization.

MarekGierlinski commented 3 months ago

Yes, the latest commit works well. Thank you. Do you know how long until it is in CRAN?

MarekGierlinski commented 3 months ago

Just to add to my original post, in a different project using targets with tar_option_set(format = "qs"), while using qs 0.26.1 I have encountered the following crash:

 *** caught segfault ***
address 0x0, cause 'memory not mapped'
/
Traceback:
 1: qs::qsave(x = object, file = path, preset = preset)
 2: store_write_path.tar_qs(store, store_convert_object(store, object),     stage)
 3: store_write_path(store, store_convert_object(store, object),     stage)
 4: store_write_object.default(target$store, target$value$object)
 5: store_write_object(target$store, target$value$object)
 6: builder_update_object(target)

It seemed to appear at random, for different targets, and I could not reproduce it in a smaller example. See also this discussion.

I'm happy to report that your latest GitHub release (0.26.3) solved the problem. So, whatever whatever you did to rectify my original issue, helped with this one too.

traversc commented 3 months ago

Out on CRAN now