Preserve namespaces as much as possible on reloads #133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hadley
reviewed
Nov 10, 2020
This way R can find the init function. Also allows some support for scanning .Call() invokations. In this case, the most recently loaded .so has precedence.
Rather than only unloading
lionel-
force-pushed
the
fix-dangling
branch
from
5eba3c7
to
306400e
Compare
jimhester
added a commit
to r-lib/cpp11
that referenced
this pull request
Jan 19, 2021
This should work better with r-lib/pkgload#133
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current reloading mechanism unloads previously loaded namespaces, including the SO / DLL. This is a source of crash in case of lingering references to the namespace environments as the native routines from unloaded SOs are no longer in memory. To work around this, pkgload updates all imports in reverse dependencies of the reloaded package. However, this workaround is not exhaustive. For instance, using
devtools::test()with the ellipsis package causes a crash because of an ellipsis function is called fromon.exit()after aload_all().With this PR,
load_all()now preserves existing namespaces as much as possible. In particular, it doesn't unload the package's shared library and keeps it loaded instead. When reloading, a copy of the SO for the new namespace is loaded from a temporary location. These temporary SOs are only unloaded on GC and deleted from their temporary location via a weak reference attached to the namespace.This mechanism ensures that lingering references to the namespace keep working as expected. Consequently the namespace propagation routine that was added to pkgload as a workaround (see #59) has been removed.
Note that
.Call()invokations that pass a string symbol rather than a structured symbol may keep crashing, because R will look into the most recently loaded SO of a given name. Since symbol registration is now the norm, I don't expect this to cause much trouble.