fix(rust/driver_manager): don't dlclose drivers #3844

Pranav2612000 · 2025-12-30T15:35:50Z

According to golang/go#11100 (comment), the -Wl,-z,nodelete does not work on macos. This causes dlclose to be called when the shared library is dropped, which results in unexpected behaviour ( I've observed the program hangs because Go cannot kill the goroutines and so waits for them to close )

On linux, since the -Wl,-z,nodelete is respected, we do not see this issue.

This PR fixes the problem by ensuring that we explicitly open the library with RTLD_NODELETE flag which ensures that dlclose is not called during unload

Fixes #3840

Pranav2612000 · 2025-12-30T16:05:56Z

Thank you @savannahar68 for debugging the issue and helping with the fix

eitsupi · 2025-12-30T16:18:11Z

rust/driver_manager/src/lib.rs

+            // 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 )
+            if cfg!(target_os = "macos") {


I suppose the cfg attribute #[cfg(...)] is better than cfg! here.
(In this case, cfg! case will compile on any OS.)

Makes sense. Updated the code to use #[cfg(...)]

amoeba · 2025-12-30T17:18:17Z

rust/driver_manager/src/lib.rs

+#[cfg(target_os = "macos")]
+use libloading::os::unix::Library;
+


Suggested change

#[cfg(target_os = "macos")]

use libloading::os::unix::Library;

I think we can remove this and just qualify the identifier below.

amoeba · 2025-12-30T17:21:05Z

rust/driver_manager/src/lib.rs

+            Library::open(
+                Some(filename.as_ref()),
+                RTLD_LAZY | RTLD_LOCAL | RTLD_NODELETE,
+            )
+            .map(Into::into)
+            .map_err(libloading_error_to_adbc_error)?


Suggested change

Library::open(

Some(filename.as_ref()),

RTLD_LAZY | RTLD_LOCAL | RTLD_NODELETE,

)

.map(Into::into)

.map_err(libloading_error_to_adbc_error)?

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.

amoeba · 2025-12-30T17:21:34Z

rust/driver_manager/src/lib.rs

+            const RTLD_LAZY: i32 = 0x1;
+            const RTLD_LOCAL: i32 = 0x4;
+            const RTLD_NODELETE: i32 = 0x80;


I guess we could pull these from the libc crate, is it worth it though? cc @eitsupi

It seems that RTLD_LAZY and RTLD_LOCAL are also defined in libloading? 🤔
https://github.com/nagisa/rust_libloading/blob/dab97c569b33bd515e16637b8dedbdc696d9ec9c/src/os/unix/consts.rs

Yeah. Only those two, but not RTLD_NODELETE. I don't feel super stressed about hard-coding them here to avoid taking on libc as a dep.

eitsupi · 2025-12-30T23:46:45Z

I don't fully understand the nature of the problem, so this may be off the mark, but is this a problem specific to the Rust Driver Manager?
Is there no need to address this in implementations in other languages?

(Just to be clear, I'm not saying everything should be addressed in this PR, just a question.)

lidavidm

Actually, I think we should just std::mem::forget the library (bypassing its Drop implementation) and not dlclose anything at all - it's just not safe. (Or remove the platform-specific cfg and do this for all platforms.)

Pranav2612000 · 2025-12-31T02:15:13Z

I don't fully understand the nature of the problem, so this may be off the mark, but is this a problem specific to the Rust Driver Manager? Is there no need to address this in implementations in other languages?

(Just to be clear, I'm not saying everything should be addressed in this PR, just a question.)

I haven't tried with other languages, but yeah, we'll probably need to do the same for other languages as well. I'm thinking of getting the rust fix in first. Doing the same for other languages should be trivial.

Pranav2612000 · 2025-12-31T02:17:21Z

Actually, I think we should just std::mem::forget the library (bypassing its Drop implementation) and not dlclose anything at all - it's just not safe. (Or remove the platform-specific cfg and do this for all platforms.)

I've tested with std:mem::forget and it works too. I'm don't have a strong preference to any approach. Do you think we should use the drop approach? I'll modify the PR to do this.

lidavidm · 2025-12-31T03:18:53Z

IIRC, we don't dlclose in the C++ driver manager, or C#. So Rust may be the only one affected.

forget does technically introduce a small leak - I think RTLD_NODELETE is preferable. Maybe (separately) we can try to get this flag defined upstream?

Pranav2612000 · 2025-12-31T08:46:47Z

Makes sense. I'll open a separate PR upstream soon to add support for RTLD_NODELETE.

Are there any other changes I should do to make this PR mergeable

lidavidm · 2025-12-31T08:48:30Z

I think we can remove the #[cfg] and just have this behavior on all platforms (maybe it doesn't work on Windows?)

Pranav2612000 · 2025-12-31T09:09:32Z

I'm not sure about windows. Let me see the current implementation and try te replicate it.

Pranav2612000 · 2025-12-31T10:47:50Z

I think we can remove the #[cfg] and just have this behavior on all platforms (maybe it doesn't work on Windows?)

Yeah. Libloading does have separate definitions for windows and unix ( https://github.com/nagisa/rust_libloading/blob/dab97c569b33bd515e16637b8dedbdc696d9ec9c/src/safe.rs#L6 ).
While we get the changes merged upstream, I'm updating this PR to mirror this - cfg conditions on whether the os is windows or unix ( rather than on mac )

Pranav2612000 · 2025-12-31T10:56:38Z

Pushed. Can you folks take another look? Meanwhile, I'll work on getting the flag, and some other improvements merged upstream to libloading

lidavidm · 2026-01-02T01:03:05Z

rust/driver_manager/src/lib.rs

+        };
+        #[cfg(not(unix))]
        let library = unsafe {
            libloading::Library::new(filename.as_ref()).map_err(libloading_error_to_adbc_error)?


The linked GitHub issue notes there's a Windows equivalent flag; is that possible to provide to libloading?

On Windows, a similar GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_PIN, ...) is required for the same effect.

I'm not a windows expert, but I see that this flag is not passed when we load the library. During loading, the LoadLibraryExW function is called ( Ref ).

The way to do it in windows is to call the GetModuleHandleExW function with this flag. libloading supports this using the pin function ( Ref ). Let's call this fn in the windows cfg, so that we have the same behaviour across all platforms?

Pushed this change for windows. Can you take a look? ~~I'm also opening up a conversation upstream to try and get this changes merged into libloading~~ Upstream discussion thread -> nagisa/rust_libloading#193

lidavidm · 2026-01-05T02:14:43Z

It seems tests are failing?

---- test_connection_cancel stdout ----

thread 'test_connection_cancel' (13755) panicked at driver_manager/tests/driver_manager_sqlite.rs:32:90:
called `Result::unwrap()` on an `Err` value: Error { message: "Error with dynamic library: invalid mode parameter", status: Internal, vendor_code: 0, sqlstate: [0, 0, 0, 0, 0], details: None }

Pranav2612000 · 2026-01-05T04:18:52Z

Weird. I tested again on mac and linux and I don't see this error. In the CI, for a test, I also see the driver not found

---- test_database_implicit_uri stdout ----

thread 'test_database_implicit_uri' (13767) panicked at driver_manager/tests/driver_manager_sqlite.rs:62:6:
called `Result::unwrap()` on an `Err` value: Error { message: "Driver not found: adbc_driver_sqlite", status: NotFound, vendor_code: 0, sqlstate: [0, 0, 0, 0, 0], details: None }

Could it be an intermittent issue with the way the driver was built? I'm trying to reproduce this error, but meanwhile can you retry the pipeline once?

Pranav2612000 · 2026-01-05T06:27:14Z

@lidavidm The tests are passing now. Here's the changes I did

Modified the RTLD_NODELETE to be of type c_int instead of i32.
Update libloading to 0.8.8 from 0.8 ( 0.8 was pinned to version 0.8.0 which did not support the pin method for windows )

lidavidm · 2026-01-05T06:58:02Z

rust/driver_manager/src/lib.rs

+        let library: libloading::Library = unsafe {
+            use std::os::raw::c_int;
+
+            const RTLD_NODELETE: c_int = 8;


It appears this is different on my system:

/usr/include/x86_64-linux-gnu/bits/dlfcn.h 41:#define RTLD_NODELETE 0x01000

It appears to be 0x80 on macOS. Where did 8 come from?

Maybe we need some #[cfg] to define this, and fall back to 0xFFFFFFFF on Unix-like platforms besides linux (glibc) and macOS?

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 ?

I'd rather not spam the console.

lidavidm · 2026-01-05T07:54:42Z

rust/driver_manager/src/lib.rs

+            )) {
+                0x1000
+            } else {
+                0xFFFF


Also wait, shouldn't this be 0 since we're OR-ing it? (Sorry, I messed this up)

Yup. I also missed it. Fixed now.

Updates libloading to 0.8.8

lidavidm

Thanks!

Pranav2612000 · 2026-01-07T04:36:46Z

Thank you for merging this @lidavidm .
Do you have a rough estimate on when the crate will be updated?

lidavidm · 2026-01-07T05:18:45Z

Hopefully this week (because you happened to coincide with the release plans)

Pranav2612000 requested a review from wjones127 as a code owner December 30, 2025 15:35

github-actions bot added this to the ADBC Libraries 22 milestone Dec 30, 2025

Pranav2612000 mentioned this pull request Dec 30, 2025

rust driver_manager: initializing the driver creates some dangling resources which hangs application during shutdown #3840

Closed

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch from 5fdead7 to 4570524 Compare December 30, 2025 16:13

eitsupi reviewed Dec 30, 2025

View reviewed changes

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch from 4570524 to 0bfa253 Compare December 30, 2025 16:46

amoeba reviewed Dec 30, 2025

View reviewed changes

lidavidm reviewed Dec 31, 2025

View reviewed changes

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch from 0bfa253 to 252ccd3 Compare December 31, 2025 10:55

lidavidm reviewed Jan 2, 2026

View reviewed changes

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch from fd2fe3c to 97055bd Compare January 2, 2026 05:49

Pranav2612000 mentioned this pull request Jan 2, 2026

feature request: add method to dlclose without unloading the library nagisa/rust_libloading#193

Open

lidavidm approved these changes Jan 5, 2026

View reviewed changes

lidavidm changed the title ~~feat(rust/driver_manager): prevent dlclose call on macos~~ feat(rust/driver_manager): don't dlclose drivers Jan 5, 2026

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch 4 times, most recently from 475d31e to 137b2a4 Compare January 5, 2026 05:48

lidavidm reviewed Jan 5, 2026

View reviewed changes

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch from 950483e to eb5873f Compare January 5, 2026 07:53

lidavidm reviewed Jan 5, 2026

View reviewed changes

Pranav2612000 added 2 commits January 5, 2026 13:25

feat(rust/driver_manager): prevent dlclose call on macos

9685749

feat(rust/driver_manager): pin to not unload lib on windows

d0d2c94

Updates libloading to 0.8.8

Pranav2612000 force-pushed the fix/fix-hang-during-dlopen-mac branch from eb5873f to d0d2c94 Compare January 5, 2026 07:56

lidavidm approved these changes Jan 6, 2026

View reviewed changes

lidavidm changed the title ~~feat(rust/driver_manager): don't dlclose drivers~~ fix(rust/driver_manager): don't dlclose drivers Jan 6, 2026

lidavidm merged commit 5a88ac3 into apache:main Jan 6, 2026
9 checks passed

		#[cfg(target_os = "macos")]
		use libloading::os::unix::Library;

fix(rust/driver_manager): don't dlclose drivers #3844

fix(rust/driver_manager): don't dlclose drivers #3844

Uh oh!

Conversation

Pranav2612000 commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pranav2612000 commented Dec 30, 2025

Uh oh!

eitsupi Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eitsupi commented Dec 30, 2025

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

Pranav2612000 commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pranav2612000 commented Dec 31, 2025

Uh oh!

lidavidm commented Dec 31, 2025

Uh oh!

Pranav2612000 commented Dec 31, 2025

Uh oh!

lidavidm commented Dec 31, 2025

Uh oh!

Pranav2612000 commented Dec 31, 2025

Uh oh!

Pranav2612000 commented Dec 31, 2025

Uh oh!

Pranav2612000 commented Dec 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pranav2612000 Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidavidm commented Jan 5, 2026

Uh oh!

Pranav2612000 commented Jan 5, 2026

Uh oh!

Pranav2612000 commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pranav2612000 Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Pranav2612000 commented Jan 7, 2026

Pranav2612000 commented Dec 30, 2025 •

edited

Loading

eitsupi Dec 30, 2025 •

edited

Loading

Pranav2612000 commented Dec 31, 2025 •

edited

Loading

Pranav2612000 Jan 2, 2026 •

edited

Loading

Pranav2612000 Jan 5, 2026 •

edited

Loading