r-lib / keyring

:closed_lock_with_key: Access the system credential store from R
https://keyring.r-lib.org/
Other
196 stars 28 forks source link

keyring_wincred_enumerate not resolved #73

Closed r2evans closed 3 years ago

r2evans commented 5 years ago

Occasionally, I get the following error:

keyring::key_list()
# Error in .Call("keyring_wincred_enumerate", filter) : 
#   "keyring_wincred_enumerate" not resolved from current namespace (keyring)

getNamespaceExports("keyring")
#  [1] "key_list"               "key_get_raw"            "keyring_unlock"         "backend"               
#  [5] "backend_env"            "key_set_with_raw_value" "key_set"                "key_delete"            
#  [9] "backend_macos"          "keyring_delete"         "key_get"                "keyring_is_locked"     
# [13] "keyring_create"         "has_keyring_support"    "backend_file"           "backend_wincred"       
# [17] "default_backend"        "keyring_list"           "key_set_with_value"     "backend_secret_service"
# [21] "backend_keyrings"       "keyring_lock"          

The only time I can get it to happen is after I've hibernated the laptop and resumed. It doesn't happen every time. The only fix I've been able to find has been to re-start R. Notably, this does not work:

keyring::key_list()
# Error in .Call("keyring_wincred_enumerate", filter) : 
#   "keyring_wincred_enumerate" not resolved from current namespace (keyring)
dlls <- getLoadedDLLs()
dlls[ "keyring" ]
#                                                           Filename Dynamic.Lookup
# keyring c:/Users/r2/R/win-library/3.5/keyring/libs/x64/keyring.dll          FALSE

library.dynam.unload("keyring", "c:/Users/r2/R/win-library/3.5/keyring", verbose=TRUE)
# now dyn.unload("c:/Users/r2/R/win-library/3.5/keyring/libs/x64/keyring.dll") ...
dlls <- getLoadedDLLs()
dlls[ "keyring" ] # empty

keyring::key_list()
# Error in .Call("keyring_wincred_enumerate", filter) : 
#   "keyring_wincred_enumerate" not resolved from current namespace (keyring)
dlls <- getLoadedDLLs()
dlls[ "keyring" ] # empty

library.dynam("keyring", "keyring", "c:/Users/r2/R/win-library/3.5", verbose=TRUE)
# now dyn.load("c:/Users/r2/R/win-library/3.5/keyring/libs/x64/keyring.dll") ...
dlls <- getLoadedDLLs()
dlls[ "keyring" ]
#                                                           Filename Dynamic.Lookup
# keyring c:/Users/r2/R/win-library/3.5/keyring/libs/x64/keyring.dll          FALSE
keyring::key_list()
# Error in .Call("keyring_wincred_enumerate", filter) : 
#   "keyring_wincred_enumerate" not resolved from current namespace (keyring)
session info ```r sessionInfo() # R version 3.5.3 (2019-03-11) # Platform: x86_64-w64-mingw32/x64 (64-bit) # Running under: Windows 10 x64 (build 17763) # Matrix products: default # locale: # [1] LC_COLLATE=English_United States.1252 LC_CTYPE=English_United States.1252 LC_MONETARY=English_United States.1252 # [4] LC_NUMERIC=C LC_TIME=English_United States.1252 # attached base packages: # [1] stats graphics grDevices utils datasets methods base # other attached packages: # [1] keyring_1.1.0 logger_0.1 data.table_1.12.0 ggplot2_3.1.0 # [5] tidyr_0.8.2 dplyr_0.8.0.1 purrr_0.2.5 tibble_2.0.1 # [9] testthat_2.0.0 # loaded via a namespace (and not attached): # [1] fs_1.2.6 usethis_1.4.0 bit64_0.9-7 lubridate_1.7.4 devtools_2.0.1 # [6] RColorBrewer_1.1-2 rprojroot_1.3-2 tools_3.5.3 backports_1.1.2 rredis_1.7.0 # [11] utf8_1.1.4 R6_2.4.0 DT_0.5 rpart_4.1-13 Hmisc_4.2-0 # [16] rgeos_0.3-28 DBI_1.0.0 lazyeval_0.2.1 colorspace_1.3-2 nnet_7.3-12 # [21] withr_2.1.2 sp_1.3-1 tidyselect_0.2.5 gridExtra_2.3 prettyunits_1.0.2 # [26] processx_3.1.0 bit_1.1-14 compiler_3.5.3 cli_1.0.1 formatR_1.5 # [31] htmlTable_1.12 flextable_0.4.5 xml2_1.2.0 officer_0.3.2 desc_1.2.0 # [36] scales_1.0.0 checkmate_1.9.1 readr_1.1.1 odbc_1.1.6 callr_2.0.4 # [41] commonmark_1.7 stringr_1.4.0 digest_0.6.18 foreign_0.8-71 rmarkdown_1.11 # [46] base64enc_0.1-3 pkgconfig_2.0.2 htmltools_0.3.6 sessioninfo_1.1.1 htmlwidgets_1.3 # [51] rlang_0.3.1 rstudioapi_0.9.0 zoo_1.8-3 jsonlite_1.6 config_0.3 # [56] zip_1.0.0 acepack_1.4.1 magrittr_1.5 Formula_1.2-3 futile.logger_1.4.3 # [61] Matrix_1.2-15 fansi_0.4.0 Rcpp_1.0.0 munsell_0.5.0 gdtools_0.1.7 # [66] yaml_2.2.0 stringi_1.3.1 pkgbuild_1.0.2 plyr_1.8.4 blob_1.1.1 # [71] grid_3.5.3 pls_2.7-0 crayon_1.3.4 lattice_0.20-38 splines_3.5.3 # [76] hms_0.4.2 knitr_1.20 pillar_1.3.1 uuid_0.1-2 pkgload_1.0.2 # [81] futile.options_1.0.1 glue_1.3.0 evaluate_0.11 latticeExtra_0.6-28 lambda.r_1.2.3 # [86] remotes_2.0.2 gtable_0.2.0 assertthat_0.2.0 roxygen2_6.1.1 violinplot_0.5.2.9000 # [91] survival_2.43-3 memoise_1.1.0 cluster_2.0.7-1 tableHTML_1.1.0 ztable_0.2.0 ```

I have not been able to define a clear reproducible pathway to get the error: many times, a hibernate/restart does not trigger the error. But every time it triggers, it has been post restart from a hibernation.

When it works, I see:

getDLLRegisteredRoutines(dlls$keyring)
#                       .Call .Call.numParameters
# 1       keyring_wincred_get                   1
# 2    keyring_wincred_exists                   1
# 3       keyring_wincred_set                   4
# 4    keyring_wincred_delete                   1
# 5 keyring_wincred_enumerate                   1

(I just restarted R, so now it's working ... I'll get the error again and come back and show what this command does while failing.)

r2evans commented 5 years ago

Update: it is not necessarily hibernate-related. I don't know what triggered it, but now it's giving me the same error. None of the above code is any different, including the last getDLLRegisteredRoutines.

One thing that I have done in the last few minutes was to devtools::load_all a couple of local packages. None of the packages I re-loaded directly reference keyring; both packages depend on another package (installed "normally" though admittedly not from CRAN) that does reference keyring. I don't know how the reload step would have triggered something, but ... I'll let you know if/when I find something new.

gaborcsardi commented 5 years ago

Interesting. I have never seen that before. I will try to poke around....

r2evans commented 5 years ago

Not to beat a dying (not-dead-yet) horse, but it just triggered with repeated devtools::load_all (the only thing I've found common with the above attempts) and no hibernate within 7 hours. Still on keyring-1.1.0, is there another windows-centric way of debugging what is going on with loaded DLLs? (I cannot imagine what devtools:: would be doing that would trigger it, so that may be a red herring. idk)

gaborcsardi commented 5 years ago

It may as well be a devtools / pkgload issue, because they play around with DLLs, and such.

r2evans commented 5 years ago

Okay, I'll see if I can trigger a fault solely based on load_all. (My guess is that it is perhaps load_all triggering some otherwise-minor memory reference bug in the keyring's C code or windows' credentials side.)

r2evans commented 5 years ago
traceback()
# 8: b_wincred_i_enumerate(filter)
# 7: b_wincred_list(self, private, service, keyring)
# 6: default_backend()$list(service, keyring = keyring)
# 5: keyring::key_list(svc)
# 4: nrow(keys <- keyring::key_list(svc))
# 3: (function (..., config_files = NA, use_parent = TRUE, active = Sys.getenv("R_CONFIG_ACTIVE"), 
#        include_password = FALSE, strict = c("error", "warning", 
#            "quiet")) 
#     ...

@gaborcsardi does this shed any light? I'd really like to help find a reprex for this.

gaborcsardi commented 5 years ago

Not really, sorry. :(

One thing I can do is using proper objects in .Call() instead of the string literals of the C function names, then the search for the C function is different, and there is a chance that this bug goes away.

r2evans commented 5 years ago

Okay, based on that comment and reading https://cran.r-project.org/doc/manuals/r-release/R-exts.html#useDynLib, I'll try something:

diff --git a/NAMESPACE b/NAMESPACE
index 7fd15a5..d6df4a7 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -29,4 +29,4 @@ importFrom(assertthat,has_name)
 importFrom(utils,URLdecode)
 importFrom(utils,head)
 importFrom(utils,tail)
-useDynLib(keyring, .registration = TRUE, .fixes = "c_")
+useDynLib(keyring, keyring_wincred_delete, keyring_wincred_enumerate, keyring_wincred_exists, keyring_wincred_get, keyring_wincred_set, .registration = TRUE, .fixes = "c_")
diff --git a/R/backend-wincred.R b/R/backend-wincred.R
index 5a9a04d..d8d77d8 100644
--- a/R/backend-wincred.R
+++ b/R/backend-wincred.R
@@ -8,24 +8,24 @@ b_wincred_protocol_version <- "1.0.0"
 ## This is a low level API

 b_wincred_i_get <- function(target) {
-  .Call("keyring_wincred_get", target)
+  .Call(keyring_wincred_get, target)
 }

 b_wincred_i_set <- function(target, password, username = NULL,
                                  session = FALSE) {
-  .Call("keyring_wincred_set", target, password, username, session)
+  .Call(keyring_wincred_set, target, password, username, session)
 }

 b_wincred_i_delete <- function(target) {
-  .Call("keyring_wincred_delete", target)
+  .Call(keyring_wincred_delete, target)
 }

 b_wincred_i_exists <- function(target) {
-  .Call("keyring_wincred_exists", target)
+  .Call(keyring_wincred_exists, target)
 }

 b_wincred_i_enumerate <- function(filter) {
-  .Call("keyring_wincred_enumerate", filter)
+  .Call(keyring_wincred_enumerate, filter)
 }

 b_wincred_i_escape <- function(x) {

I tried initially to do the same for macos funcs as well, but when compiling it complained about

*** arch - i386
Error: package or namespace load failed for 'keyring' in FUN(X[[i]], ...):
 no such symbol keyring_macos_create in package C:/Users/r2/R/win-library/3.5/keyring/libs/i386/keyring.dll
Error: loading failed

so I'm not explicitly loading them at the moment. Initial installation and use is functional, I'll see if I can ever retrigger the event and report back. If I don't see anything for "some time", I'll provide a PR (though I don't know if the macos error can be fixed from my end, if you have advice on that I'd appreciate it).

gaborcsardi commented 5 years ago

I tried initially to do the same for macos funcs as well, but when compiling it complained about

Yeah, that's why I haven't done this, I guess.

(though I don't know if the macos error can be fixed from my end, if you have advice on that I'd appreciate it).

I think we'll need to create dummy functions on the "other" archs.

r2evans commented 5 years ago

If the dynamic symbol lookup is working just fine for the other services, is there a reason we cannot keep the non-wincred services as-is for now?

gaborcsardi commented 5 years ago

We would still need to create dummy windows functions on the other platforms, and once the machinery for this works, we might as well do this the same way on all platforms.

r2evans commented 4 years ago

As a follow-up, I've been using a modified version (with my patch above) since September and cannot remember the last time I saw a wincred error. While this neither addresses the macos stubs issue nor proves it is perfect on windows, it does suggest (at least on my system) improved stability.

kgruschow commented 3 years ago
library(checkpoint) # This is built against Microsoft R 4.0.2
checkpoint("2020-11-05") # Using Packages from November 5th, 2020, which notably means CRAN 4.0.3
# This is done because data.table fcase is very efficient and was released
# after 4.0.2 
library(keyring) #used to store/retrieve keys like passwords for SQL
key_set("kgruschow_server", "kgruschow")

this has been giving me the keyring_wincred_enumerate error about 90% of the time after multiple fresh starts. system does not hibernate, however from the above, I'd tend to think checkpoint may use the load tools described above.

It'd be great to somehow release the patch, it's workflow breaking for me at the moment. I've never seen this before when running CRAN/4.0.3 main, and in testing the above with current R, excluding checkpoint, it all works... thus far.

later edit: As mentioned when running MRAN 4.0.2 and Nov 5 2020 Checkpoint, encountered error most of the time but not always.

I switched to CRAN 4.0.3 and not loading checkpoint, and not seen the error after using it extensively.

gaborcsardi commented 3 years ago

@kgruschow Of course this works for me...

What version of version are you on?

r2evans commented 3 years ago

@gaborcsardi, updated status: still using my patch from above, I have not seen any problems at all. And since I use it daily with sleeps and hibernates and such, that seems (to me) to be a good indication of (qualified) success.

@kgruschow, have you tried my patch above? I just cloned the repo locally, applied that patch, then devtools::build(".") and install.packages("keyring_1.1.0.tar.gz"). I haven't see a problem since that patch. It is a windows-only patch at the moment, until gaborcsardi is able to find a more elegant/consistent workaround for this problem.

gaborcsardi commented 3 years ago

Hopefully #102 fixes this, feedback is most welcome!

r2evans commented 3 years ago

(Initial install and startup works, no surprised. Time will tell, I'll be running this version for a while and will report back. Thank you @gaborcsardi !)

r2evans commented 3 years ago

FYI, I've been using r-lib/keyring@c91e87d since late April and have seen zero repeats of this bug. (Sorry for the late reply, I'm installing 1.2 now.)

Thank you!

gaborcsardi commented 3 years ago

Great, thanks for letting me know!