From 0ede7525678482e8e2b63c295f8a8ecde38b592e Mon Sep 17 00:00:00 2001 From: Dirk Eddelbuettel Date: Sat, 11 Nov 2023 15:45:11 -0600 Subject: [PATCH] Correct filehandle use in VFS serialization --- R/RcppExports.R | 4 ++++ R/VFS.R | 23 +++++++++++++---------- inst/include/tiledb.h | 6 +++++- src/RcppExports.cpp | 11 +++++++++++ src/libtiledb.cpp | 26 ++++++++++++++++++++------ 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/R/RcppExports.R b/R/RcppExports.R index 4f344b717d..1ee4aeedc9 100644 --- a/R/RcppExports.R +++ b/R/RcppExports.R @@ -954,6 +954,10 @@ libtiledb_vfs_copy_file <- function(vfs, old_uri, new_uri) { .Call(`_tiledb_libtiledb_vfs_copy_file`, vfs, old_uri, new_uri) } +libtiledb_vfs_fh_free <- function(fhxp) { + invisible(.Call(`_tiledb_libtiledb_vfs_fh_free`, fhxp)) +} + libtiledb_stats_enable <- function() { invisible(.Call(`_tiledb_libtiledb_stats_enable`)) } diff --git a/R/VFS.R b/R/VFS.R index c2a1ad5ba8..db943fded1 100644 --- a/R/VFS.R +++ b/R/VFS.R @@ -394,9 +394,10 @@ tiledb_vfs_unserialize <- function(uri, vfs = tiledb_get_vfs()) { stopifnot("Argument 'vfs' must a tiledb_vfs object" = is(vfs, "tiledb_vfs"), "Argument 'uri' must be character" = is.character(uri)) n <- tiledb_vfs_file_size(uri) - file <- tiledb_vfs_open(uri, "READ") - vec <- tiledb_vfs_read(file, 0, n) - tiledb_vfs_close(file) + fh <- tiledb_vfs_open(uri, "READ") + vec <- tiledb_vfs_read(fh, 0, n) + tiledb_vfs_close(fh) + libtiledb_vfs_fh_free(fh) ## The gzcon(rawConnection()) idea is from https://stackoverflow.com/a/58136567/508431 ## The packBits(intToBits()) part on the int vector read is from a friend via slack obj <- unserialize(gzcon(rawConnection(packBits(intToBits(vec))))) @@ -420,15 +421,17 @@ tiledb_vfs_serialize <- function(obj, uri, vfs = tiledb_get_vfs()) { saveRDS(obj, tf) ## Read local file - file <- tiledb_vfs_open(tf, "READ") - vec <- tiledb_vfs_read(file, 0, tiledb_vfs_file_size(tf)) - tiledb_vfs_close(file) + fh <- tiledb_vfs_open(tf, "READ") + vec <- tiledb_vfs_read(fh, 0, tiledb_vfs_file_size(tf)) + tiledb_vfs_close(fh) + libtiledb_vfs_fh_free(fh) ## Now write 'vec' to the target URI - file <- tiledb_vfs_open(uri, "WRITE") - tiledb_vfs_write(file, vec) - tiledb_vfs_sync(file) - tiledb_vfs_close(file) + fh <- tiledb_vfs_open(uri, "WRITE") + tiledb_vfs_write(fh, vec) + tiledb_vfs_sync(fh) + tiledb_vfs_close(fh) + libtiledb_vfs_fh_free(fh) unlink(tf) invisible(uri) diff --git a/inst/include/tiledb.h b/inst/include/tiledb.h index 61a6c276b3..58e9b8f158 100644 --- a/inst/include/tiledb.h +++ b/inst/include/tiledb.h @@ -76,7 +76,11 @@ typedef std::unordered_map> m // C++ compiler complains about missing delete functionality when we use tiledb_vfs_fh_t directly struct vfs_fh { - void *fh; +#if TILEDB_VERSION_MAJOR == 2 && TILEDB_VERSION_MINOR >= 15 + tiledb_vfs_fh_handle_t* fh; +#else + tiledb_vfs_fh_t* fh; +#endif }; typedef struct vfs_fh vfs_fh_t; diff --git a/src/RcppExports.cpp b/src/RcppExports.cpp index 2b2d9b67c6..11992520bd 100644 --- a/src/RcppExports.cpp +++ b/src/RcppExports.cpp @@ -2847,6 +2847,16 @@ BEGIN_RCPP return rcpp_result_gen; END_RCPP } +// libtiledb_vfs_fh_free +void libtiledb_vfs_fh_free(XPtr fhxp); +RcppExport SEXP _tiledb_libtiledb_vfs_fh_free(SEXP fhxpSEXP) { +BEGIN_RCPP + Rcpp::RNGScope rcpp_rngScope_gen; + Rcpp::traits::input_parameter< XPtr >::type fhxp(fhxpSEXP); + libtiledb_vfs_fh_free(fhxp); + return R_NilValue; +END_RCPP +} // libtiledb_stats_enable void libtiledb_stats_enable(); RcppExport SEXP _tiledb_libtiledb_stats_enable() { @@ -3783,6 +3793,7 @@ static const R_CallMethodDef CallEntries[] = { {"_tiledb_libtiledb_vfs_dir_size", (DL_FUNC) &_tiledb_libtiledb_vfs_dir_size, 2}, {"_tiledb_libtiledb_vfs_ls", (DL_FUNC) &_tiledb_libtiledb_vfs_ls, 2}, {"_tiledb_libtiledb_vfs_copy_file", (DL_FUNC) &_tiledb_libtiledb_vfs_copy_file, 3}, + {"_tiledb_libtiledb_vfs_fh_free", (DL_FUNC) &_tiledb_libtiledb_vfs_fh_free, 1}, {"_tiledb_libtiledb_stats_enable", (DL_FUNC) &_tiledb_libtiledb_stats_enable, 0}, {"_tiledb_libtiledb_stats_disable", (DL_FUNC) &_tiledb_libtiledb_stats_disable, 0}, {"_tiledb_libtiledb_stats_reset", (DL_FUNC) &_tiledb_libtiledb_stats_reset, 0}, diff --git a/src/libtiledb.cpp b/src/libtiledb.cpp index 2cb930f97f..e1c5a66362 100644 --- a/src/libtiledb.cpp +++ b/src/libtiledb.cpp @@ -4324,11 +4324,15 @@ XPtr libtiledb_vfs_open(XPtr ctxxp, XPtr check_xptr_tag(vfsxp); std::shared_ptr ctx = ctxxp.get()->ptr(); std::shared_ptr vfs = vfsxp.get()->ptr(); +#if TILEDB_VERSION >= TileDB_Version(2,15,0) + tiledb_vfs_fh_handle_t *fh = nullptr; +#else tiledb_vfs_fh_t *fh = nullptr; +#endif tiledb_vfs_mode_t vfsmode = _string_to_tiledb_vfs_mode_t(mode); tiledb_vfs_open(ctx.get(), vfs.get(), uri.c_str(), vfsmode, &fh); XPtr ptr = make_xptr(new vfs_fh_t); - ptr->fh = static_cast(fh); + ptr->fh = fh; return ptr; } @@ -4337,7 +4341,7 @@ void libtiledb_vfs_close(XPtr ctxxp, XPtr fh) { check_xptr_tag(ctxxp); check_xptr_tag(fh); std::shared_ptr ctx = ctxxp.get()->ptr(); - tiledb_vfs_close(ctx.get(), static_cast(fh->fh)); + tiledb_vfs_close(ctx.get(), fh->fh); } // [[Rcpp::export]] @@ -4346,8 +4350,7 @@ void libtiledb_vfs_write(XPtr ctxxp, XPtr fh, check_xptr_tag(ctxxp); check_xptr_tag(fh); std::shared_ptr ctx = ctxxp.get()->ptr(); - tiledb_vfs_write(ctx.get(), static_cast(fh->fh), - &(vec[0]), vec.size()*sizeof(int)); + tiledb_vfs_write(ctx.get(), fh->fh, &(vec[0]), vec.size()*sizeof(int)); } // [[Rcpp::export]] @@ -4359,7 +4362,7 @@ Rcpp::IntegerVector libtiledb_vfs_read(XPtr ctxxp, XPtr(fh->fh), offs, &(buf[0]), nb); + tiledb_vfs_read(ctx.get(), fh->fh, offs, &(buf[0]), nb); return buf; } @@ -4368,7 +4371,7 @@ void libtiledb_vfs_sync(XPtr ctxxp, XPtr fh) { check_xptr_tag(ctxxp); check_xptr_tag(fh); std::shared_ptr ctx = ctxxp.get()->ptr(); - tiledb_vfs_sync(ctx.get(), static_cast(fh->fh)); + tiledb_vfs_sync(ctx.get(), fh->fh); } // [[Rcpp::export]] @@ -4390,6 +4393,17 @@ std::string libtiledb_vfs_copy_file(XPtr vfs, std::string old_uri, return new_uri; } +// [[Rcpp::export]] +void libtiledb_vfs_fh_free(XPtr fhxp) { + check_xptr_tag(fhxp); + spdl::trace("[libtiledb_vfs_clear_handle] entered"); + vfs_fh_t* strptr = fhxp.get(); +#if TILEDB_VERSION >= TileDB_Version(2,15,0) + tiledb_vfs_fh_handle_t *fh = strptr->fh; + tiledb_vfs_fh_free(&fh); +#endif +} + /** * Stats */