amoeba commented on code in PR #3844:
URL: https://github.com/apache/arrow-adbc/pull/3844#discussion_r2653492189
##########
rust/driver_manager/src/lib.rs:
##########
@@ -381,6 +384,22 @@ impl ManagedDriver {
let default_entrypoint = get_default_entrypoint(filename.as_ref());
let entrypoint = entrypoint.unwrap_or(default_entrypoint.as_bytes());
+ // By default, go builds the libraries with '-Wl -z nodelete' which
does not
+ // unload the go runtime. This isn't respected on mac (
https://github.com/golang/go/issues/11100#issuecomment-932638093 )
+ // so we need to explicitly load the library with RTLD_NODELETE( which
prevents unloading )
+ #[cfg(target_os = "macos")]
+ let library: libloading::Library = unsafe {
+ const RTLD_LAZY: i32 = 0x1;
+ const RTLD_LOCAL: i32 = 0x4;
+ const RTLD_NODELETE: i32 = 0x80;
Review Comment:
I guess we could pull these from the `libc` crate, is it worth it though? cc
@eitsupi
##########
rust/driver_manager/src/lib.rs:
##########
@@ -381,6 +384,22 @@ impl ManagedDriver {
let default_entrypoint = get_default_entrypoint(filename.as_ref());
let entrypoint = entrypoint.unwrap_or(default_entrypoint.as_bytes());
+ // By default, go builds the libraries with '-Wl -z nodelete' which
does not
+ // unload the go runtime. This isn't respected on mac (
https://github.com/golang/go/issues/11100#issuecomment-932638093 )
+ // so we need to explicitly load the library with RTLD_NODELETE( which
prevents unloading )
+ #[cfg(target_os = "macos")]
+ let library: libloading::Library = unsafe {
+ const RTLD_LAZY: i32 = 0x1;
+ const RTLD_LOCAL: i32 = 0x4;
+ const RTLD_NODELETE: i32 = 0x80;
+ Library::open(
+ Some(filename.as_ref()),
+ RTLD_LAZY | RTLD_LOCAL | RTLD_NODELETE,
+ )
+ .map(Into::into)
+ .map_err(libloading_error_to_adbc_error)?
Review Comment:
```suggestion
libloading::os::unix::Library::open(
Some(filename.as_ref()),
RTLD_LAZY | RTLD_LOCAL | RTLD_NODELETE,
)
.map_err(libloading_error_to_adbc_error)?
.into()
```
This feels a little simpler and more idiomatic.
##########
rust/driver_manager/src/lib.rs:
##########
@@ -118,6 +118,9 @@ use arrow_array::ffi_stream::{ArrowArrayStreamReader,
FFI_ArrowArrayStream};
use arrow_array::{Array, RecordBatch, RecordBatchReader, StructArray};
use toml::de::DeTable;
+#[cfg(target_os = "macos")]
+use libloading::os::unix::Library;
+
Review Comment:
```suggestion
```
I think we can remove this and just qualify the identifier below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
