Skip to content

Commit f1bf94d

Browse files
authored
cargo-credential-libsecret: load libsecret only once (#15295)
Previously, libsecret was repeatedly loaded and unloaded—at least twice during cargo login (once for action get and once for action login). This caused issues where calls could hang indefinitely due to glib failing to clean up properly (some threads still running after unloading) and generating numerous error messages: ``` cargo login --registry ownreg Updating `ownreg` index please paste the token for ownreg below (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: cannot register existing type 'SecretService' (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: cannot add private field to invalid (non-instantiatable) type '<invalid>' (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: g_type_add_interface_static: assertion 'G_TYPE_IS_INSTANTIATABLE (instance_type)' failed (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: g_type_add_interface_static: assertion 'G_TYPE_IS_INSTANTIATABLE (instance_type)' failed (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: cannot register existing type 'SecretBackend' (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: g_type_interface_add_prerequisite: assertion 'G_TYPE_IS_INTERFACE (interface_type)' failed (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: g_type_interface_add_prerequisite: assertion 'G_TYPE_IS_INTERFACE (interface_type)' failed (process:100727): GLib-CRITICAL **: 08:59:54.568: g_once_init_leave_pointer: assertion 'result != 0' failed (process:100727): GLib-GObject-CRITICAL **: 08:59:54.568: g_type_add_interface_static: assertion 'G_TYPE_IS_INSTANTIATABLE (instance_type)' failed ``` Now, libsecret is stored in a OnceLock, ensuring it is only loaded once, preventing unnecessary unload/reload cycles. Fixes #15603
2 parents 41cf907 + b881a32 commit f1bf94d

File tree

5 files changed

+51
-18
lines changed

5 files changed

+51
-18
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ blake3 = "1.5.5"
2727
build-rs = { version = "0.3.1", path = "crates/build-rs" }
2828
cargo = { path = "" }
2929
cargo-credential = { version = "0.4.2", path = "credential/cargo-credential" }
30-
cargo-credential-libsecret = { version = "0.4.15", path = "credential/cargo-credential-libsecret" }
30+
cargo-credential-libsecret = { version = "0.5.0", path = "credential/cargo-credential-libsecret" }
3131
cargo-credential-macos-keychain = { version = "0.4.15", path = "credential/cargo-credential-macos-keychain" }
3232
cargo-credential-wincred = { version = "0.4.15", path = "credential/cargo-credential-wincred" }
3333
cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" }

credential/cargo-credential-libsecret/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "cargo-credential-libsecret"
3-
version = "0.4.15"
3+
version = "0.5.0"
44
rust-version = "1.87" # MSRV:1
55
edition.workspace = true
66
license.workspace = true

credential/cargo-credential-libsecret/src/lib.rs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ mod linux {
8383
...
8484
) -> *mut gchar;
8585

86-
pub struct LibSecretCredential;
86+
pub struct LibSecretCredential {
87+
libsecret: Library,
88+
}
8789

8890
fn label(index_url: &str) -> CString {
8991
CString::new(format!("cargo-registry:{}", index_url)).unwrap()
@@ -105,31 +107,41 @@ mod linux {
105107
}
106108
}
107109

108-
impl Credential for LibSecretCredential {
110+
impl LibSecretCredential {
111+
pub fn new() -> Result<LibSecretCredential, Error> {
112+
// Dynamically load libsecret to avoid users needing to install
113+
// additional -dev packages when building this provider.
114+
let libsecret = unsafe { Library::new("libsecret-1.so.0") }.context(
115+
"failed to load libsecret: try installing the `libsecret` \
116+
or `libsecret-1-0` package with the system package manager",
117+
)?;
118+
Ok(Self { libsecret })
119+
}
120+
}
121+
122+
impl Credential for &LibSecretCredential {
109123
fn perform(
110124
&self,
111125
registry: &RegistryInfo<'_>,
112126
action: &Action<'_>,
113127
_args: &[&str],
114128
) -> Result<CredentialResponse, Error> {
115-
// Dynamically load libsecret to avoid users needing to install
116-
// additional -dev packages when building this provider.
117-
let lib;
118129
let secret_password_lookup_sync: Symbol<'_, SecretPasswordLookupSync>;
119130
let secret_password_store_sync: Symbol<'_, SecretPasswordStoreSync>;
120131
let secret_password_clear_sync: Symbol<'_, SecretPasswordClearSync>;
121132
unsafe {
122-
lib = Library::new("libsecret-1.so.0").context(
123-
"failed to load libsecret: try installing the `libsecret` \
124-
or `libsecret-1-0` package with the system package manager",
125-
)?;
126-
secret_password_lookup_sync = lib
133+
secret_password_lookup_sync = self
134+
.libsecret
127135
.get(b"secret_password_lookup_sync\0")
128136
.map_err(Box::new)?;
129-
secret_password_store_sync =
130-
lib.get(b"secret_password_store_sync\0").map_err(Box::new)?;
131-
secret_password_clear_sync =
132-
lib.get(b"secret_password_clear_sync\0").map_err(Box::new)?;
137+
secret_password_store_sync = self
138+
.libsecret
139+
.get(b"secret_password_store_sync\0")
140+
.map_err(Box::new)?;
141+
secret_password_clear_sync = self
142+
.libsecret
143+
.get(b"secret_password_clear_sync\0")
144+
.map_err(Box::new)?;
133145
}
134146

135147
let index_url_c = CString::new(registry.index_url).unwrap();

src/cargo/util/auth/mod.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,27 @@ static BUILT_IN_PROVIDERS: &[&'static str] = &[
508508
"cargo:libsecret",
509509
];
510510

511+
/// Retrieves a cached instance of `LibSecretCredential`.
512+
/// Must be cached to avoid repeated load/unload cycles, which are not supported by `glib`.
513+
#[cfg(target_os = "linux")]
514+
fn get_credential_libsecret(
515+
) -> CargoResult<&'static cargo_credential_libsecret::LibSecretCredential> {
516+
static CARGO_CREDENTIAL_LIBSECRET: std::sync::OnceLock<
517+
cargo_credential_libsecret::LibSecretCredential,
518+
> = std::sync::OnceLock::new();
519+
// Unfortunately `get_or_try_init` is not yet stable. This workaround is not threadsafe but
520+
// loading libsecret twice will only temporary increment the ref counter, which is decrement
521+
// again when `drop` is called.
522+
match CARGO_CREDENTIAL_LIBSECRET.get() {
523+
Some(lib) => Ok(lib),
524+
None => {
525+
let _ = CARGO_CREDENTIAL_LIBSECRET
526+
.set(cargo_credential_libsecret::LibSecretCredential::new()?);
527+
Ok(CARGO_CREDENTIAL_LIBSECRET.get().unwrap())
528+
}
529+
}
530+
}
531+
511532
fn credential_action(
512533
gctx: &GlobalContext,
513534
sid: &SourceId,
@@ -545,7 +566,7 @@ fn credential_action(
545566
#[cfg(target_os = "macos")]
546567
"cargo:macos-keychain" => Box::new(cargo_credential_macos_keychain::MacKeychain {}),
547568
#[cfg(target_os = "linux")]
548-
"cargo:libsecret" => Box::new(cargo_credential_libsecret::LibSecretCredential {}),
569+
"cargo:libsecret" => Box::new(get_credential_libsecret()?),
549570
name if BUILT_IN_PROVIDERS.contains(&name) => {
550571
Box::new(cargo_credential::UnsupportedCredential {})
551572
}

0 commit comments

Comments
 (0)