Skip to content

Commit 1ca1faf

Browse files
authored
Fix SubprocessServer cache thread-safety and test isolation (#38501)
1 parent 4c4a2c1 commit 1ca1faf

3 files changed

Lines changed: 17 additions & 6 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"comment": "Modify this file in a trivial way to cause this test suite to run",
3+
"revision": 1
4+
}

sdks/python/apache_beam/transforms/external_test.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,13 @@ def test_implicit_builder_with_constructor_method(self):
799799

800800
class JavaJarExpansionServiceTest(unittest.TestCase):
801801
def setUp(self):
802-
SubprocessServer._cache._live_owners = set()
802+
# Temporarily override _live_owners with an empty set for this test,
803+
# preventing contamination of the process-wide global cache and avoiding
804+
# side effects on other tests.
805+
patcher = mock.patch.object(
806+
SubprocessServer._cache, '_live_owners', new=set())
807+
patcher.start()
808+
self.addCleanup(patcher.stop)
803809

804810
def test_classpath(self):
805811
with tempfile.TemporaryDirectory() as temp_dir:

sdks/python/apache_beam/utils/subprocess_server.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,14 @@ def __init__(self, constructor, destructor):
7878
self._counter = 0
7979

8080
def _next_id(self):
81-
with self._lock:
82-
self._counter += 1
83-
return self._counter
81+
# Caller must hold self._lock.
82+
self._counter += 1
83+
return self._counter
8484

8585
def register(self):
86-
owner = self._next_id()
87-
self._live_owners.add(owner)
86+
with self._lock:
87+
owner = self._next_id()
88+
self._live_owners.add(owner)
8889
return owner
8990

9091
def purge(self, owner):

0 commit comments

Comments
 (0)