Skip to content

Conversation

@vladvildanov
Copy link
Collaborator

@vladvildanov vladvildanov commented Dec 23, 2025

Description of change

Please provide a description of the change here.

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?

PR fixes a race condition between loop.call_later() and loop.run_forever() methods, so the renewal task is scheduled before the event loop is actually running

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a potential race condition in the token manager's start() method that could occur when scheduling callbacks before the event loop thread was fully initialized. The fix improves thread-safety by using proper synchronization primitives and thread-safe scheduling methods.

Key Changes:

  • Replaced asyncio.Event with threading.Event for cross-thread synchronization to properly coordinate between the main thread and event loop thread
  • Changed from loop.call_later() to loop.call_soon_threadsafe() for thread-safe callback scheduling
  • Simplified the _renew_token() method signature by removing the init_event parameter and handling initialization signaling through a wrapper function
Comments suppressed due to low confidence (1)

redis/auth/token_manager.py:310

  • The _renew_token method signature has been updated to remove the init_event parameter, but the async version _renew_token_async at line 310 still retains this parameter. This creates an inconsistency between the two methods. For consistency and maintainability, consider whether _renew_token_async should also be updated to remove the init_event parameter if it's no longer needed, or document why the async version still requires it.
    ):
        """

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vladvildanov and others added 2 commits December 23, 2025 14:02
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ The following Jit checks failed to run:

  • secret-detection-trufflehog

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

More info in the Jit platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants