Skip to content

Consolidating to asyncio.Lock over threading.Lock#442

Merged
rodrigobr-msft merged 8 commits into
mainfrom
users/robrandao/lock
Jul 10, 2026
Merged

Consolidating to asyncio.Lock over threading.Lock#442
rodrigobr-msft merged 8 commits into
mainfrom
users/robrandao/lock

Conversation

@rodrigobr-msft

Copy link
Copy Markdown
Contributor

This pull request updates the in-memory storage and transcript store implementations to use asyncio.Lock and async with for concurrency control instead of the standard threading Lock. This change ensures that locking is compatible with asynchronous code, preventing potential deadlocks or blocking in async environments.

Concurrency model update:

  • Replaced threading.Lock with asyncio.Lock in both memory_storage.py and transcript_memory_store.py to ensure proper synchronization in async contexts. [1] [2]

  • Updated all critical sections in MemoryStorage methods (read, write, delete) to use async with self._lock: instead of with self._lock:. [1] [2] [3]

  • Updated all critical sections in TranscriptMemoryStore methods (log_activity, get_transcript_activities, delete_transcript, list_transcripts) to use async with self.lock: instead of with self.lock:. [1] [2] [3] [4]

Copilot AI review requested due to automatic review settings July 7, 2026 15:00
@rodrigobr-msft
rodrigobr-msft marked this pull request as ready for review July 7, 2026 15:01
@rodrigobr-msft
rodrigobr-msft requested a review from a team as a code owner July 7, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the in-memory storage implementations to use asyncio.Lock/async with instead of threading.Lock/with, aligning concurrency control with the project’s async execution model and avoiding blocking the event loop.

Changes:

  • Switched MemoryStorage to use asyncio.Lock and async with for read/write/delete critical sections.
  • Switched TranscriptMemoryStore to use asyncio.Lock and async with for transcript mutation and queries.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/memory_storage.py Replaces sync locking with asyncio.Lock and async context management around in-memory state access.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/transcript_memory_store.py Replaces sync locking with asyncio.Lock and async context management around transcript operations.
Comments suppressed due to low confidence (1)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/memory_storage.py:33

  • Inside MemoryStorage.read(), target_cls is validated as non-None before the lock, so the later if not target_cls: branch is unreachable. This dead branch also forces result to be typed as dict[str, StoreItem] even though the method returns dict[str, StoreItemT]. Simplify the branch and align the type annotation.
        async with self._lock:
            for key in keys:
                if key == "":
                    raise ValueError("MemoryStorage.read(): key cannot be empty")
                if key in self._memory:

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

Copilot AI review requested due to automatic review settings July 7, 2026 15:06
@rodrigobr-msft
rodrigobr-msft enabled auto-merge (squash) July 7, 2026 15:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/memory_storage.py:33

  • In an async context, it’s better to validate keys before awaiting on the lock. As written, an invalid key can still acquire the lock first, unnecessarily blocking other coroutines.
        async with self._lock:
            for key in keys:
                if key == "":
                    raise ValueError("MemoryStorage.read(): key cannot be empty")
                if key in self._memory:

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/memory_storage.py:55

  • In an async context, validate inputs before awaiting on the lock. This avoids acquiring the lock (and potentially blocking other tasks) when the request is invalid.
        async with self._lock:
            for key in changes:
                if key == "":
                    raise ValueError("MemoryStorage.write(): key cannot be empty")
                self._memory[key] = changes[key].store_item_to_json()

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/memory_storage.py:65

  • In an async context, validate inputs before awaiting on the lock. This prevents invalid input from taking the lock and briefly blocking other coroutines.
        async with self._lock:
            for key in keys:
                if key == "":
                    raise ValueError("MemoryStorage.delete(): key cannot be empty")
                if key in self._memory:

Copilot AI review requested due to automatic review settings July 8, 2026 20:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings July 10, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings July 10, 2026 17:43
@rodrigobr-msft
rodrigobr-msft merged commit 4a37fa0 into main Jul 10, 2026
10 of 11 checks passed
@rodrigobr-msft
rodrigobr-msft deleted the users/robrandao/lock branch July 10, 2026 17:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

# Licensed under the MIT License.

from threading import Lock
from asyncio import Lock
Comment on lines +15 to 16
This class is async-safe and stores all activities in a list. It supports logging activities,
retrieving activities for a specific channel and conversation, and filtering by timestamp.
Copilot AI review requested due to automatic review settings July 10, 2026 17:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

An in-memory implementation of the TranscriptLogger for storing and retrieving activities.

This class is thread-safe and stores all activities in a list. It supports logging activities,
This class is async-safe and stores all activities in a list. It supports logging activities,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect use of threading. Lock in Async Methods (MemoryStorage, TranscriptMemoryStore)

3 participants