RISK: unserialize() warning on "cannot unserialize ALTVEC object ..." should be an error

[HenrikBengtsson]

Issue

It is possible to create RDS files that will silently materialize with empty data portions when read back into R. All we get is a warning. For example, this may happen when the object saved to file holds an internal ALTVEC object of a class defined in a third-party package, and that package is no longer installed when the file is read back in again. Technically, this happens in the serialize()/unserialize() implementation in base R.

How likely is it that this can happen in reality? Here are a few cases I can imagine:

1. Data with ALTVEC-representation is saved to an RDS file in R 4.5.3. User upgrades to R 4.6.0, continues to work with the data in the RDS file forgetting to install the required R package.

2. A user saves such an RDS file with a collaborator that does not have the required package installed.

3. A user parallelize using a ALTVEC-based object where the parallel workers do not have the required package installed.

One could argue that one should never save such data to file, or parallel in that way, but we know that mistakes happens, and they will eventually happen, especially when there are many players (packages and users) involved.

Personally, I'm very conservative when it comes working with data. Anything that could silently produce incorrect results, or be lost in a non-blocking warning, is high-risk behavior by my books. I've seen way too many cases of this in real-world, scientific projects. If we're lucky we catch it early, sometimes we catch it weeks or months later, but in the worst-case scenario we never detect these mistakes. Ultimately, data mistakes can risk patients' safety, e.g. see the Duke/Potter clinical-trial case (which eventually involved more than mistakes).

Minimal reproducible example

We start by created a custom ALTVEC-vector object on file. In this example I use 'mori', but this problem can happen with any serialized custom ALTREP implementation.

# R --vanilla --quiet
x <- rnorm(1e6)
y <- mori::share(x)
str(y)
#> num [1:1000000] -0.251 0.165 -0.758 -0.155 1.857 ...
saveRDS(y, file = "altvec.rds")  ## creates a teeny file (< 100 bytes)
quit(save = "no")

Next, I start a fresh new R session where I make sure the 'mori' package is no longer available (see above use-case examples).

.libPaths(tempdir())  ## Emulate 'mori' not being available
z <- readRDS("altvec.rds")
#> Warning message:
#> In readRDS("altvec.rds") :
#>   cannot unserialize ALTVEC object of class 'mori_real' from package 'mori'; #> returning length zero vector
str(z)
#>  num(0) 

Note how the data is now an empty vector! If we don't catch that warning, this may result in incorrect downstream results.

Troubleshooting

The "cannot unserialize ALTVEC object" warning is produced by the internal ALTREP_UNSERIALIZE_EX() function in base R;

attribute_hidden SEXP
ALTREP_UNSERIALIZE_EX(SEXP info, SEXP state, SEXP attr, int objf, int levs)
{
    SEXP csym = ALTREP_SERIALIZED_CLASS_CLSSYM(info);
    SEXP psym = ALTREP_SERIALIZED_CLASS_PKGSYM(info);
    int type = ALTREP_SERIALIZED_CLASS_TYPE(info);

    /* look up the class in the registry and handle failure */
    SEXP class = ALTREP_UNSERIALIZE_CLASS(info);
    if (class == NULL) {
	switch(type) {
	case LGLSXP:
        ...
	case EXPRSXP:
	    warning("cannot unserialize ALTVEC object of class '%s' from "
		    "package '%s'; returning length zero vector",
		    CHAR(PRINTNAME(csym)), CHAR(PRINTNAME(psym)));
	    return allocVector(type, 0);
	default:
	    error("cannot unserialize this ALTREP object");
	}
    }
    ...
}    

Suggestion

I'd like to argue that the current behavior is never wanted and that warning should be escalated to an error, i.e.

	    error("cannot unserialize ALTVEC object of class '%s' from "
		    "package '%s'; returning length zero vector",
		    CHAR(PRINTNAME(csym)), CHAR(PRINTNAME(psym)));

Alternatively, provide a mechanism to allow for a warning, but make the error the default.

Workarounds

With the current behavior, we have a few options to protect us against this issue, but it requires that we are aware of it.

1. Escalate all warnings to errors (always)

We can use R option warn to escalate all warnings to become errors, e.g.

options(warn = 2)
z <- readRDS("altvec.rds")
Error in readRDS("altvec.rds") : 
  (converted from warning) cannot unserialize ALTVEC object of class 'mori_real' from package 'mori'; returning length zero vector

2. Escalate all warnings to errors (per call)

We can use tryCatch() to escalate any warning to an error per call, e.g.

z <- tryCatch({
  readRDS("altvec.rds")
}, warning = stop)
#> Error in readRDS("altvec.rds") : 
#>  cannot unserialize ALTVEC object of class 'mori_real' from package 'mori'; returning length zero vector

3. Escalate select warnings to errors (per call)

We can inspect a warning, here the warning message, an escalate it to an error if it we think it's important, e.g.

# Did we received a "cannot unserialize ALTVEC" warning?
escalate_unserialize_ALTVEC_warning <- function(w) {
  ## From src/main/altrep.c (Note, this message is currently untranslated, i.e. it's always in English)
  msgfmt <- "cannot unserialize ALTVEC object of class '%s' from package '%s'; returning length zero vector"
  pattern <- sprintf("^%s$", gsub("%s", "[[:alnum:]_]+", msgfmt))
  if (grepl(pattern, conditionMessage(w))) stop(w) 
}

z <- withCallingHandlers({
  readRDS("altvec.rds")
}, warning = escalate_unserialize_ALTVEC_warning)
#> Error in readRDS("altvec.rds") : 
#>  cannot unserialize ALTVEC object of class 'mori_real' from package 'mori'; returning length zero vector

4. Escalate select warnings to errors (for all calls)

As above, but we register a global calling handler to look out for this problem, e.g.

globalCallingHandlers(warning = escalate_unserialize_ALTVEC_warning)

z <- readRDS("altvec.rds")
#> Error in readRDS("altvec.rds") : 
#>   cannot unserialize ALTVEC object of class 'mori_real' from package 'mori'; returning length zero vector

WARNING: There's always a risk that someone overrides this global calling handler.

5. Override readRDS() with a conservative version

Less robust, but could of course also override readRDS(), e.g.

readRDS <- function(...) {
  withCallingHandlers({
    base::readRDS(...)
  }, warning = escalate_unserialize_ALTVEC_warning)
}

z <- readRDS("altvec.rds")
#> Error in base::readRDS(...) : 
#>  cannot unserialize ALTVEC object of class 'mori_real' from package 'mori'; returning length zero vector