From e208691f95eaab1b6b6c4a1209fb9c9a970b0543 Mon Sep 17 00:00:00 2001 From: Benjamin Bigler Date: Wed, 4 Jun 2025 17:22:27 +0200 Subject: [PATCH 1/3] chore: bump cargo-credential-libsecret --- Cargo.lock | 2 +- Cargo.toml | 2 +- credential/cargo-credential-libsecret/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0240af562..ef57c42ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -407,7 +407,7 @@ dependencies = [ [[package]] name = "cargo-credential-libsecret" -version = "0.4.15" +version = "0.5.0" dependencies = [ "anyhow", "cargo-credential", diff --git a/Cargo.toml b/Cargo.toml index da3d6bf9a..489905638 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ blake3 = "1.5.5" build-rs = { version = "0.3.1", path = "crates/build-rs" } cargo = { path = "" } cargo-credential = { version = "0.4.2", path = "credential/cargo-credential" } -cargo-credential-libsecret = { version = "0.4.15", path = "credential/cargo-credential-libsecret" } +cargo-credential-libsecret = { version = "0.5.0", path = "credential/cargo-credential-libsecret" } cargo-credential-macos-keychain = { version = "0.4.15", path = "credential/cargo-credential-macos-keychain" } cargo-credential-wincred = { version = "0.4.15", path = "credential/cargo-credential-wincred" } cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" } diff --git a/credential/cargo-credential-libsecret/Cargo.toml b/credential/cargo-credential-libsecret/Cargo.toml index 53cd0ea60..a691996db 100644 --- a/credential/cargo-credential-libsecret/Cargo.toml +++ b/credential/cargo-credential-libsecret/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-credential-libsecret" -version = "0.4.15" +version = "0.5.0" rust-version = "1.87" # MSRV:1 edition.workspace = true license.workspace = true From 22940c701e36cb44f533c5c3b6fd7d3ba0f5141a Mon Sep 17 00:00:00 2001 From: Benjamin Bigler Date: Wed, 4 Jun 2025 15:59:24 +0200 Subject: [PATCH 2/3] refactor: make LibSecretCredential own libsecret library --- .../cargo-credential-libsecret/src/lib.rs | 38 ++++++++++++------- src/cargo/util/auth/mod.rs | 2 +- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/credential/cargo-credential-libsecret/src/lib.rs b/credential/cargo-credential-libsecret/src/lib.rs index 0d10fb836..7feefc267 100644 --- a/credential/cargo-credential-libsecret/src/lib.rs +++ b/credential/cargo-credential-libsecret/src/lib.rs @@ -83,7 +83,9 @@ mod linux { ... ) -> *mut gchar; - pub struct LibSecretCredential; + pub struct LibSecretCredential { + libsecret: Library, + } fn label(index_url: &str) -> CString { CString::new(format!("cargo-registry:{}", index_url)).unwrap() @@ -105,6 +107,18 @@ mod linux { } } + impl LibSecretCredential { + pub fn new() -> Result { + // Dynamically load libsecret to avoid users needing to install + // additional -dev packages when building this provider. + let libsecret = unsafe { Library::new("libsecret-1.so.0") }.context( + "failed to load libsecret: try installing the `libsecret` \ + or `libsecret-1-0` package with the system package manager", + )?; + Ok(Self { libsecret }) + } + } + impl Credential for LibSecretCredential { fn perform( &self, @@ -112,24 +126,22 @@ mod linux { action: &Action<'_>, _args: &[&str], ) -> Result { - // Dynamically load libsecret to avoid users needing to install - // additional -dev packages when building this provider. - let lib; let secret_password_lookup_sync: Symbol<'_, SecretPasswordLookupSync>; let secret_password_store_sync: Symbol<'_, SecretPasswordStoreSync>; let secret_password_clear_sync: Symbol<'_, SecretPasswordClearSync>; unsafe { - lib = Library::new("libsecret-1.so.0").context( - "failed to load libsecret: try installing the `libsecret` \ - or `libsecret-1-0` package with the system package manager", - )?; - secret_password_lookup_sync = lib + secret_password_lookup_sync = self + .libsecret .get(b"secret_password_lookup_sync\0") .map_err(Box::new)?; - secret_password_store_sync = - lib.get(b"secret_password_store_sync\0").map_err(Box::new)?; - secret_password_clear_sync = - lib.get(b"secret_password_clear_sync\0").map_err(Box::new)?; + secret_password_store_sync = self + .libsecret + .get(b"secret_password_store_sync\0") + .map_err(Box::new)?; + secret_password_clear_sync = self + .libsecret + .get(b"secret_password_clear_sync\0") + .map_err(Box::new)?; } let index_url_c = CString::new(registry.index_url).unwrap(); diff --git a/src/cargo/util/auth/mod.rs b/src/cargo/util/auth/mod.rs index 2dc66f562..546602d48 100644 --- a/src/cargo/util/auth/mod.rs +++ b/src/cargo/util/auth/mod.rs @@ -545,7 +545,7 @@ fn credential_action( #[cfg(target_os = "macos")] "cargo:macos-keychain" => Box::new(cargo_credential_macos_keychain::MacKeychain {}), #[cfg(target_os = "linux")] - "cargo:libsecret" => Box::new(cargo_credential_libsecret::LibSecretCredential {}), + "cargo:libsecret" => Box::new(cargo_credential_libsecret::LibSecretCredential::new()?), name if BUILT_IN_PROVIDERS.contains(&name) => { Box::new(cargo_credential::UnsupportedCredential {}) } From b881a3285b0c7796e3b30b94829e05d7fea856f6 Mon Sep 17 00:00:00 2001 From: Benjamin Bigler Date: Wed, 4 Jun 2025 16:28:24 +0200 Subject: [PATCH 3/3] fix: add OnceLock for LibSecretCredential Previously, LibSecretCredential was created multiple times and with it libsecret was loaded and unloaded multiple times, leading to issues where calls could run indefinitely due to glib not properly cleaning up. Now, LibSecretCredential is stored in a OnceLock, ensuring it is only created once, preventing unnecessary unload/reload cycles of libsecret. --- .../cargo-credential-libsecret/src/lib.rs | 2 +- src/cargo/util/auth/mod.rs | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/credential/cargo-credential-libsecret/src/lib.rs b/credential/cargo-credential-libsecret/src/lib.rs index 7feefc267..684f0f8da 100644 --- a/credential/cargo-credential-libsecret/src/lib.rs +++ b/credential/cargo-credential-libsecret/src/lib.rs @@ -119,7 +119,7 @@ mod linux { } } - impl Credential for LibSecretCredential { + impl Credential for &LibSecretCredential { fn perform( &self, registry: &RegistryInfo<'_>, diff --git a/src/cargo/util/auth/mod.rs b/src/cargo/util/auth/mod.rs index 546602d48..0ce9d4140 100644 --- a/src/cargo/util/auth/mod.rs +++ b/src/cargo/util/auth/mod.rs @@ -508,6 +508,27 @@ static BUILT_IN_PROVIDERS: &[&'static str] = &[ "cargo:libsecret", ]; +/// Retrieves a cached instance of `LibSecretCredential`. +/// Must be cached to avoid repeated load/unload cycles, which are not supported by `glib`. +#[cfg(target_os = "linux")] +fn get_credential_libsecret( +) -> CargoResult<&'static cargo_credential_libsecret::LibSecretCredential> { + static CARGO_CREDENTIAL_LIBSECRET: std::sync::OnceLock< + cargo_credential_libsecret::LibSecretCredential, + > = std::sync::OnceLock::new(); + // Unfortunately `get_or_try_init` is not yet stable. This workaround is not threadsafe but + // loading libsecret twice will only temporary increment the ref counter, which is decrement + // again when `drop` is called. + match CARGO_CREDENTIAL_LIBSECRET.get() { + Some(lib) => Ok(lib), + None => { + let _ = CARGO_CREDENTIAL_LIBSECRET + .set(cargo_credential_libsecret::LibSecretCredential::new()?); + Ok(CARGO_CREDENTIAL_LIBSECRET.get().unwrap()) + } + } +} + fn credential_action( gctx: &GlobalContext, sid: &SourceId, @@ -545,7 +566,7 @@ fn credential_action( #[cfg(target_os = "macos")] "cargo:macos-keychain" => Box::new(cargo_credential_macos_keychain::MacKeychain {}), #[cfg(target_os = "linux")] - "cargo:libsecret" => Box::new(cargo_credential_libsecret::LibSecretCredential::new()?), + "cargo:libsecret" => Box::new(get_credential_libsecret()?), name if BUILT_IN_PROVIDERS.contains(&name) => { Box::new(cargo_credential::UnsupportedCredential {}) }