From 96c4a41882dfdad3d00efb0f35e301b2bf55f092 Mon Sep 17 00:00:00 2001 From: lysuhher Date: Sat, 1 Aug 2026 18:02:02 +0300 Subject: [PATCH] fix(ndb): avoid unbound lock in delete callback --- .../google/cloud/ndb/_datastore_api.py | 16 ++++++------ .../tests/unit/test__datastore_api.py | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/packages/google-cloud-ndb/google/cloud/ndb/_datastore_api.py b/packages/google-cloud-ndb/google/cloud/ndb/_datastore_api.py index 6e210cccf787..d3f4c49ea251 100644 --- a/packages/google-cloud-ndb/google/cloud/ndb/_datastore_api.py +++ b/packages/google-cloud-ndb/google/cloud/ndb/_datastore_api.py @@ -434,6 +434,7 @@ def delete(key, options): use_global_cache = context._use_global_cache(key, options) use_datastore = context._use_datastore(key, options) transaction = context.transaction + lock = None if use_global_cache: cache_key = _cache.global_cache_key(key) @@ -450,17 +451,18 @@ def delete(key, options): yield batch.delete(key) if use_global_cache: - if transaction: + if lock: + if transaction: - def callback(): - _cache.global_unlock_for_write(cache_key, lock).result() + def callback(): + _cache.global_unlock_for_write(cache_key, lock).result() - context.call_on_transaction_complete(callback) + context.call_on_transaction_complete(callback) - elif use_datastore: - yield _cache.global_unlock_for_write(cache_key, lock) + else: + yield _cache.global_unlock_for_write(cache_key, lock) - else: + elif not use_datastore: yield _cache.global_delete(cache_key) diff --git a/packages/google-cloud-ndb/tests/unit/test__datastore_api.py b/packages/google-cloud-ndb/tests/unit/test__datastore_api.py index 443e8733032f..17509b996646 100644 --- a/packages/google-cloud-ndb/tests/unit/test__datastore_api.py +++ b/packages/google-cloud-ndb/tests/unit/test__datastore_api.py @@ -921,6 +921,31 @@ def test_without_datastore(Batch, global_cache): assert global_cache.get([cache_key]) == [None] + @staticmethod + @mock.patch("google.cloud.ndb._datastore_api._NonTransactionalCommitBatch") + def test_without_datastore_with_transaction(Batch, global_cache): + context = context_module.get_context() + callbacks = [] + + with context.new( + transaction=b"abc123", transaction_complete_callbacks=callbacks + ).use(): + key = key_module.Key("SomeKind", 1) + cache_key = _cache.global_cache_key(key._key) + global_cache.set({cache_key: b"foo"}) + + batch = Batch.return_value + batch.delete.side_effect = Exception("Shouldn't use Datastore") + + future = _api.delete( + key._key, + _options.Options(use_datastore=False), + ) + assert future.result() is None + + assert callbacks == [] + assert global_cache.get([cache_key]) == [None] + @staticmethod @mock.patch("google.cloud.ndb._datastore_api._NonTransactionalCommitBatch") def test_cache_disabled(Batch, global_cache):