Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo-credential-libsecret: load libsecret only once #15295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

biglben
Copy link

@biglben biglben commented Mar 11, 2025

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.

Previously, libsecret was loaded and unloaded multiple times, leading
to issues where calls could run indefinitely due to glib not properly
cleaning up.
Now, libsecret is stored in a OnceLock, ensuring it is only loaded once,
preventing unnecessary unload/reload cycles.
@rustbot
Copy link
Collaborator

rustbot commented Mar 11, 2025

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-credential-provider Area: credential provider for storing and retreiving credentials S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2025
@epage
Copy link
Contributor

epage commented Mar 11, 2025

The main thing I'm wondering is if this is the right layer of caching.

Alternatively, we could lazy load these fields in LibSecretCredential and use a OnceLock on that in the cargo side of the code. Or we could skip OnceLock and have a cache on GlobalContext which would give users control over lifetimes.

A potential way to side step any of that is for us to not load the library multiple times. It looks like during login() we call registry() to get Registry::new_handle() so we can call registry.host() . We might be able to split registry() into registry() and registry_cfg() and use the latter in login(), avoiding looking up the token just to throw it away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-credential-provider Area: credential provider for storing and retreiving credentials S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants