| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent d461da7 commit 0719f1e
2 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -13,24 +13,20 @@ | |||
| 13 | 13 | # limitations under the License. | |
| 14 | 14 | ||
| 15 | 15 | import abc | |
| 16 | + import threading | ||
| 16 | 17 | ||
| 17 | - from google.protobuf import duration_pb2 | ||
| 18 | - from google.protobuf import timestamp_pb2 | ||
| 19 | - from google.protobuf import field_mask_pb2 | ||
| 20 | - from google.protobuf import struct_pb2 | ||
| 21 | - from google.protobuf import wrappers_pb2 | ||
| 18 | + from google.protobuf import ( | ||
| 19 | + duration_pb2, | ||
| 20 | + field_mask_pb2, | ||
| 21 | + struct_pb2, | ||
| 22 | + timestamp_pb2, | ||
| 23 | + wrappers_pb2, | ||
| 24 | + ) | ||
| 22 | 25 | ||
| 23 | 26 | from proto.marshal import compat | |
| 24 | - from proto.marshal.collections import MapComposite | ||
| 25 | - from proto.marshal.collections import Repeated | ||
| 26 | - from proto.marshal.collections import RepeatedComposite | ||
| 27 | - | ||
| 27 | + from proto.marshal.collections import MapComposite, Repeated, RepeatedComposite | ||
| 28 | 28 | from proto.marshal.rules import bytes as pb_bytes | |
| 29 | - from proto.marshal.rules import stringy_numbers | ||
| 30 | - from proto.marshal.rules import dates | ||
| 31 | - from proto.marshal.rules import struct | ||
| 32 | - from proto.marshal.rules import wrappers | ||
| 33 | - from proto.marshal.rules import field_mask | ||
| 29 | + from proto.marshal.rules import dates, field_mask, stringy_numbers, struct, wrappers | ||
| 34 | 30 | from proto.primitives import ProtoType | |
| 35 | 31 | ||
| 36 | 32 | ||
@@ -168,7 +164,10 @@ def get_rule(self, proto_type): | |||
| 168 | 164 | # See https://github.com/googleapis/proto-plus-python/issues/349 | |
| 169 | 165 | if rule == self._noop and hasattr(self, "_instances"): | |
| 170 | 166 | for _, instance in self._instances.items(): | |
| 171 | - rule = instance._rules.get(proto_type, self._noop) | ||
| 167 | + # Avoid race condition where instance is added to _instances | ||
| 168 | + # but __init__ hasn't run yet. | ||
| 169 | + rules = getattr(instance, "_rules", {}) | ||
| 170 | + rule = rules.get(proto_type, self._noop) | ||
| 172 | 171 | if rule != self._noop: | |
| 173 | 172 | break | |
| 174 | 173 | return rule | |
@@ -254,6 +253,7 @@ class Marshal(BaseMarshal): | |||
| 254 | 253 | """ | |
| 255 | 254 | ||
| 256 | 255 | _instances = {} | |
| 256 | + _instance_creation_lock = threading.Lock() | ||
| 257 | 257 | ||
| 258 | 258 | def __new__(cls, *, name: str): | |
| 259 | 259 | """Create a marshal instance. | |
@@ -265,7 +265,18 @@ def __new__(cls, *, name: str): | |||
| 265 | 265 | """ | |
| 266 | 266 | klass = cls._instances.get(name) | |
| 267 | 267 | if klass is None: | |
| 268 | - klass = cls._instances[name] = super().__new__(cls) | ||
| 268 | + with cls._instance_creation_lock: | ||
| 269 | + # Double check inside lock to confirm another thread hasn't | ||
| 270 | + # created the instance while we were waiting for the lock. | ||
| 271 | + klass = cls._instances.get(name) | ||
| 272 | + if klass is None: | ||
| 273 | + # Use Copy-on-Write to avoid 'RuntimeError: dictionary changed size during iteration' | ||
| 274 | + # in BaseMarshal.get_rule. This allows other threads to iterate over the old | ||
| 275 | + # dictionary safely while we replace it with a new one atomically. | ||
| 276 | + new_instances = cls._instances.copy() | ||
| 277 | + klass = super().__new__(cls) | ||
| 278 | + new_instances[name] = klass | ||
| 279 | + cls._instances = new_instances | ||
| 269 | 280 | ||
| 270 | 281 | return klass | |
| 271 | 282 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,58 @@ | |||
| 1 | + from unittest.mock import patch | ||
| 2 | + | ||
| 3 | + from proto.marshal.marshal import Marshal | ||
| 4 | + | ||
| 5 | + | ||
| 6 | + def test_marshal_identity(): | ||
| 7 | + m1 = Marshal(name="foo") | ||
| 8 | + m2 = Marshal(name="foo") | ||
| 9 | + assert m1 is m2 | ||
| 10 | + | ||
| 11 | + | ||
| 12 | + def test_marshal_different_names(): | ||
| 13 | + m1 = Marshal(name="foo") | ||
| 14 | + m2 = Marshal(name="bar") | ||
| 15 | + assert m1 is not m2 | ||
| 16 | + | ||
| 17 | + | ||
| 18 | + def test_marshal_new_race_condition(): | ||
| 19 | + # Test the case where klass is None at line 266, | ||
| 20 | + # but NOT None at line 271 (another thread created it). | ||
| 21 | + | ||
| 22 | + from unittest.mock import MagicMock | ||
| 23 | + | ||
| 24 | + mock_instances = MagicMock() | ||
| 25 | + | ||
| 26 | + call_count = 0 | ||
| 27 | + | ||
| 28 | + def get_side_effect(name, default=None): | ||
| 29 | + nonlocal call_count | ||
| 30 | + call_count += 1 | ||
| 31 | + if call_count == 1: | ||
| 32 | + return None # First check returns None | ||
| 33 | + # Simulate another thread having created it | ||
| 34 | + return "fake_instance" | ||
| 35 | + | ||
| 36 | + mock_instances.get.side_effect = get_side_effect | ||
| 37 | + | ||
| 38 | + with patch.object(Marshal, "_instances", mock_instances): | ||
| 39 | + instance = Marshal(name="race_test") | ||
| 40 | + assert instance == "fake_instance" | ||
| 41 | + | ||
| 42 | + | ||
| 43 | + def test_get_rule_uninitialized_instance(): | ||
| 44 | + class FakeMarshal: | ||
| 45 | + # No _rules attribute | ||
| 46 | + pass | ||
| 47 | + | ||
| 48 | + m = Marshal(name="default") | ||
| 49 | + | ||
| 50 | + # Inject FakeMarshal into Marshal._instances safely using patch.dict | ||
| 51 | + with patch.dict(Marshal._instances, {"fake_uninitialized": FakeMarshal()}): | ||
| 52 | + | ||
| 53 | + class DummyType: | ||
| 54 | + pass | ||
| 55 | + | ||
| 56 | + # This should not raise AttributeError because of getattr safety | ||
| 57 | + rule = m.get_rule(DummyType) | ||
| 58 | + assert rule == m._noop | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments