fix(keyring): make keyring unlock thread safe

[abn]

When using poetry install, Poetry executor prefers to execute operations using a concurrent.futures.ThreadPoolExecutor. This can lead to multiple credential store unlock requests to be dispatched simultaneously.

If the credential store is locked, and a user either intentionally or unintentionally dismisses the prompt to provide the store password; or the dbus messages fail to launch the prompter the Poetry user can experience, what can appear as a "hang". This in fact can be several threads competing with each other waiting for a dbus event indefinitely; this is a consequence of how Poetry uses keyring.

This change introduces the following:

• pre-flight check for installer commands that ensures keyring unlocks only once for the duration of the command
• improved logging of keyring unlock event and/or failure
• thread safe caching of PasswordManager.<use_keyring|keyring>
• the @atomic_cached_property decorator

This change does not address the following:

• handling cases where dbus message fails or prompter is blocked
• authentication of a locked keyring in a non-tty session

I am also open to consider an alternative for @atomic_cached_property as I am not so comfortable having to implement something like this unless we really need it.

Resolves: #8623

Summary by Sourcery

Make keyring unlock thread-safe and improve logging.

Bug Fixes:

• Fix a hang that could occur when multiple threads attempt to unlock the keyring simultaneously.

Enhancements:

• Add a pre-flight check to ensure keyring unlocks only once during a command.
• Improve logging of keyring unlock events and failures.
• Introduce thread-safe caching for PasswordManager properties.

Tests:

• Update tests to reflect changes in keyring handling.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request makes the keyring unlock thread-safe to avoid multiple credential store unlock requests when using a ThreadPoolExecutor. It introduces a pre-flight check to ensure keyring unlocks only once, improves logging, and implements thread-safe caching for PasswordManager properties.

Sequence diagram for thread-safe keyring unlock process

sequenceDiagram
    participant App as Poetry Application
    participant PM as PasswordManager
    participant PK as PoetryKeyring
    participant K as Keyring System

    App->>PM: Check keyring availability
    activate PM
    PM->>PK: is_available()
    activate PK
    PK->>K: Check backend validity
    PK->>K: Test keyring unlock (get_password)
    K-->>PK: Return unlock status
    PK-->>PM: Return availability status
    deactivate PK
    PM-->>App: Return keyring status
    deactivate PM
    Note over App,K: Only one unlock attempt is made
    Note over App,K: due to thread-safe caching

Loading

Class diagram for thread-safe keyring implementation

classDiagram
    class PasswordManager {
        -Config _config
        +use_keyring: bool
        +keyring: PoetryKeyring
    }

    class PoetryKeyring {
        -str _namespace
        +get_password(name: str, username: str)
        +set_password(name: str, username: str, password: str)
        +delete_password(name: str, username: str)
        +get_entry_name(name: str)
        +is_available(): bool
    }

    class AtomicCachedProperty {
        -BoundedSemaphore _semaphore
        -dict[object, Lock] _locks
        +__get__(instance, owner)
    }

    note for AtomicCachedProperty "New thread-safe property decorator"
    PasswordManager --> PoetryKeyring
    PasswordManager ..> AtomicCachedProperty : uses

Loading

State diagram for keyring availability check

stateDiagram-v2
    [*] --> CheckingAvailability
    CheckingAvailability --> ImportCheck
    ImportCheck --> BackendCheck: Import successful
    ImportCheck --> Unavailable: Import failed
    BackendCheck --> UnlockCheck: Valid backend found
    BackendCheck --> Unavailable: No valid backend
    UnlockCheck --> Available: Unlock successful
    UnlockCheck --> Unavailable: Unlock failed
    Available --> [*]
    Unavailable --> [*]

Loading

File-Level Changes

Change	Details	Files
Make keyring unlock thread-safe	Added atomic_cached_property decorator to make use_keyring and keyring properties thread-safe. Added caching of is_available to prevent repeated checks. Improved error handling and logging during keyring access. Added pre-flight check in the application to ensure keyring unlocks only once. Updated tests to reflect changes in keyring handling and availability checks.	src/poetry/utils/password_manager.py
Add pre-flight check for keyring availability	Added a pre-flight check to determine keyring availability and log the result. Updated tests to verify pre-flight check behavior.	src/poetry/console/application.py
Update tests for keyring changes	Updated keyring fixtures to use set_keyring_backend. Added a default keyring fixture to ensure consistent keyring behavior. Updated tests to account for changes in keyring availability checks. Added tests for thread safety and caching of properties. Added a helper function to set the keyring backend and clear the availability cache.	tests/conftest.py tests/utils/test_authenticator.py tests/utils/test_password_manager.py tests/helpers.py
Implement atomic_cached_property decorator	Implemented atomic_cached_property decorator to provide thread-safe caching for properties. Added tests to verify the thread safety and caching behavior of the decorator. Updated tests to use the new decorator for thread-safe caching.	src/poetry/utils/threading.py tests/utils/test_threading.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time. You can also use
  this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @abn - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[abn]

Updated the implementation to only unlock for installer commands.