FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(proto-plus): make Marshal thread-safe and handle race conditions by chalmerlowe · Pull Request #17774 · googleapis/google-cloud-python · GitHub

fix(proto-plus): make Marshal thread-safe and handle race conditions - #17774

Merged
chalmerlowe merged 4 commits into
mainfrom
fix/marshal-thread-safety
Jul 21, 2026
Merged

fix(proto-plus): make Marshal thread-safe and handle race conditions#17774
chalmerlowe merged 4 commits into
mainfrom
fix/marshal-thread-safety

Conversation

Copy link
Copy Markdown
Contributor

Thread-safe Marshal Initialization

Problem

A RuntimeError saying dictionary changed size during iteration can occur randomly in BaseMarshal.get_rule. This happens because one thread is reading the _instances dictionary while another thread is adding a new instance to it. This is common when using features like Firestore's on_snapshot in a background thread.

Solution

  1. Thread-safe instance creation: Applied a locking mechanism in Marshal.__new__. It uses a technique called "double-checked locking" to make sure only one thread creates a new instance at a time without slowing down normal reads.
  2. Copy-on-Write Pattern: When a new instance is added, we make a copy of the existing instances dictionary, add the new one, and then replace the dictionary atomically. This ensures that threads iterating over the old dictionary are not interrupted.
  3. Defensive Attribute Access: Added safety in BaseMarshal.get_rule to avoid trying to read rules from an instance that has been registered but has not finished initializing yet.

Notes to Reviewers

  • The Copy-on-Write pattern allows us to avoid locking during reads (which are very common), keeping performance high while fixing the race condition.
  • A new test file test_marshal_thread_safety.py has been added to cover these concurrency scenarios.
  • Some portions of files in this package were reformatted automatically by the code linters.

Fixes #15100

- Use Double-Checked Locking and Copy-on-Write in Marshal.__new__

- Use getattr safely in BaseMarshal.get_rule

- Add tests for concurrency scenarios

Fixes #15100

gemini-code-assist Bot 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Code Review

This pull request introduces thread-safety improvements to the Marshal class, implementing a threading lock, double-checked locking, and copy-on-write dictionary updates to prevent race conditions and dictionary mutation errors during iteration. It also adds safety checks for uninitialized instances and a comprehensive test suite. The review feedback recommends using unittest.mock.patch.dict in the tests to safely modify Marshal._instances and avoid potential test pollution if a test fails.

chalmerlowe self-assigned this Jul 20, 2026
…tests

Follows reviewer suggestion to avoid potential test pollution.
chalmerlowe marked this pull request as ready for review July 20, 2026 16:36
chalmerlowe requested a review from a team as a code owner July 20, 2026 16:36
parthea assigned parthea and unassigned chalmerlowe Jul 20, 2026

parthea 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

chalmerlowe merged commit 0719f1e into main Jul 21, 2026
39 checks passed
chalmerlowe deleted the fix/marshal-thread-safety branch July 21, 2026 09:30
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

RuntimeError: dictionary changed size during iteration

2 participants


Back | FazBrowse Home | New Git URL