Skip to content

Conversation

@felipecrv
Copy link
Contributor

@felipecrv felipecrv commented May 6, 2025

Since the removal of the global driver lock, there is no risk of deadlocks between the locks on the driver and on the database objects.

IIUC, this was why member functions on the database struct took a mutable self parameter:

    /// Initialize the given database using the loaded driver.
    ///
    /// This uses `&mut self` to prevent a deadlock.
    fn database_init(
        &mut self,
        mut database: ffi::FFI_AdbcDatabase,
    ) -> Result<ffi::FFI_AdbcDatabase> {

Requiring &mut self is overkill and requires another layer of locking at the application side if multiple threads share a database instance (as they should for connection pooling).

This doesn't mean we are passing a forged mutable pointer to the driver via the FFI, the locking on the inner database ensures exclusive access to the FFI_AdbcDatabase.

    /// Initialize the given connection using the loaded driver.
    fn connection_init(
        &self,
        mut connection: ffi::FFI_AdbcConnection,
    ) -> Result<ffi::FFI_AdbcConnection> {
        let driver = self.ffi_driver();
        let mut database = self.inner.database.lock().unwrap();

        // ConnectionInit
        let mut error = ffi::FFI_AdbcError::with_driver(driver);
        let method = driver_method!(driver, ConnectionInit);
        let status = unsafe { method(&mut connection, &mut *database, &mut error) };
        check_status(status, error)?;

        Ok(connection)
    }

This might be considered a breaking change.

Closes #2790

@felipecrv felipecrv requested a review from wjones127 as a code owner May 6, 2025 18:11
@github-actions github-actions bot added this to the ADBC Libraries 19 milestone May 6, 2025
@felipecrv felipecrv changed the title feat(rust): Let immutable drivers create connections fix(rust): Let immutable drivers create connections May 6, 2025
@lidavidm lidavidm changed the title fix(rust): Let immutable drivers create connections fix(rust)!: Let immutable drivers create connections May 6, 2025
@felipecrv felipecrv requested a review from mbrobbel May 7, 2025 00:13
@felipecrv felipecrv merged commit ed2fe87 into apache:main May 7, 2025
9 checks passed
@felipecrv felipecrv deleted the const-ref-db branch May 7, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rust: Unable to create a connection from a non-mut &Database

2 participants