Pranav2612000 commented on code in PR #3844:
URL: https://github.com/apache/arrow-adbc/pull/3844#discussion_r2660537301
##########
rust/driver_manager/src/lib.rs:
##########
@@ -381,8 +381,31 @@ impl ManagedDriver {
let default_entrypoint = get_default_entrypoint(filename.as_ref());
let entrypoint = entrypoint.unwrap_or(default_entrypoint.as_bytes());
- let library = unsafe {
-
libloading::Library::new(filename.as_ref()).map_err(libloading_error_to_adbc_error)?
+ // 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(unix)]
+ let library: libloading::Library = unsafe {
+ use std::os::raw::c_int;
+
+ const RTLD_NODELETE: c_int = 8;
Review Comment:
Yup. You're right. It's `0x80` for mac ( Ref ->
https://github.com/realthunder/mac-headers/blob/88d4348d732fe95385faef950eb660626f9ff711/usr/include/dlfcn.h#L72
)
> Maybe we need some #[cfg] to define this, and fall back to 0xFFFFFFFF on
Unix-like platforms besides linux (glibc) and macOS?
Yeah. Doing this. Or do you think we should print error/warning and not 'OR'
with `RTLD_NODELETE` for unknown OS ? Something like
[this](https://github.com/nagisa/rust_libloading/blob/dab97c569b33bd515e16637b8dedbdc696d9ec9c/src/os/unix/consts.rs#L193)
?
##########
rust/driver_manager/src/lib.rs:
##########
@@ -381,8 +381,31 @@ impl ManagedDriver {
let default_entrypoint = get_default_entrypoint(filename.as_ref());
let entrypoint = entrypoint.unwrap_or(default_entrypoint.as_bytes());
- let library = unsafe {
-
libloading::Library::new(filename.as_ref()).map_err(libloading_error_to_adbc_error)?
+ // 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(unix)]
+ let library: libloading::Library = unsafe {
+ use std::os::raw::c_int;
+
+ const RTLD_NODELETE: c_int = 8;
Review Comment:
Yup. You're right. It's `0x80` for mac ( Ref ->
https://github.com/realthunder/mac-headers/blob/88d4348d732fe95385faef950eb660626f9ff711/usr/include/dlfcn.h#L72
)
> Maybe we need some #[cfg] to define this, and fall back to 0xFFFFFFFF on
Unix-like platforms besides linux (glibc) and macOS?
Yeah. Doing this. Or do you think we should print error/warning and not 'OR'
with `RTLD_NODELETE` for unknown OS ? Something like
[this](https://github.com/nagisa/rust_libloading/blob/dab97c569b33bd515e16637b8dedbdc696d9ec9c/src/os/unix/consts.rs#L193)
?
--
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]
