brodieG / unitizer

Easy R Unit Tests
Other
39 stars 6 forks source link

Unloading Namespaces with Class Unions Causes problems? #259

Closed brodieG closed 4 years ago

brodieG commented 5 years ago

Possibly related to #110.

Trying to come up with a more reproducible result, but in the meantime, this is a unitizer that was created with among other things, diffobj on the search path. As best I can tall any slot that is of a class that overlaps with class unions from packages that were unloaded cause validations on failure.

Browse[2]> xx <- readRDS(file.path(test.dir, 'fastlm1.unitizer', 'data.rds'))
Browse[2]> str(xx, 1)
Formal class 'unitizer' [package "unitizer"] with 35 slots
Browse[2]> validObject(xx, complete=TRUE)
[1] TRUE
Browse[2]> search()
 [1] ".GlobalEnv"        "package:utzflm"    "package:diffobj"  
 [4] "package:unitizer"  "package:testthat"  "package:stats"    
 [7] "package:graphics"  "package:grDevices" "package:utils"    
[10] "package:datasets"  "package:methods"   "Autoloads"        
[13] "package:base"     
Browse[2]> detach('package:diffobj', unload=TRUE)
Browse[2]> validObject(xx, complete=TRUE)
Error in validObject(object[[x]], complete = TRUE) : 
  invalid class "unitizerItem" object: 1: In slot "call" of class "NULL": superclass "charOrNULL" not defined in the environment of the object's class
invalid class "unitizerItem" object: 2: In slot "call.dep" of class "character": superclass "charOrNULL" not defined in the environment of the object's class
invalid class "unitizerItem" object: 3: In slot "id" of class "integer": superclass "doAutoCOrInt" not defined in the environment of the object's class
invalid class "unitizerItem" object: 4: In slot "ls" of class "data.frame": In slot "names" of class "character": superclass "charOrNULL" not defined in the environment of the object's class
invalid class "unitizerItem" object: 5: In slot "ls" of class "data.frame": In slot "row.names" of class "integer": superclass "doAutoCOrInt" not defined in the environment of the object's class
invalid class "unitizerItem" object: 6: In slot "ls" of class "data.frame": In slot ".S3Class" of class "character": superclass "charOrNULL" not 
brodieG commented 5 years ago

Possibly related: https://github.com/r-lib/devtools/issues/168 https://github.com/r-lib/devtools/pull/223 https://github.com/r-lib/pkgload/issues/46

brodieG commented 5 years ago

Problem seems to be that detach calls methods:::cacheMetaData in detach mode, which then calls:

                .uncacheClass(cl, cldef)
                .removeSuperclassBackRefs(cl, cldef, searchWhere)

But neither of these force re-caching of the subclasses. .removeSuperclassBackRefs removes references in the superclass this contains, whereas we need the opposite, to remove the references from the classes that contain it. Maybe something like:

                .uncacheClass(cl, cldef)
                .removeSuperclassBackRefs(cl, cldef, searchWhere)
                .recacheSubclasses(def@className, def, doSubclasses, env)

Would do the trick? Actually nope, it won't. That just adds references, it does not remove them.

brodieG commented 5 years ago

So figured out the problem, as described here: https://stat.ethz.ch/pipermail/r-devel/2019-January/077206.html

Next step is probably to add calls to removeClass for every class union in every namespace that is successfully unloaded. While the call to removeClass itself will fail due to the rm at the end on the locked namespace, most of the prior steps that do what we need should happen.

brodieG commented 4 years ago

Unfortunately I didn't resolve this back then, but I believe this is fixed based on R3.6.0 NEWS:

      \item Class unions are unloaded when their namespace is
      unloaded (\PR{17531}, adapted from a patch by Brodie Gaslam).