You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by tv...@apache.org on 2022/02/28 08:33:30 UTC
[buildstream] branch tristan/fix-leaking-grpc-threads created (now 02f6a13)
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a change to branch tristan/fix-leaking-grpc-threads
in repository https://gitbox.apache.org/repos/asf/buildstream.git.
at 02f6a13 sandbox/_sandboxremote.py: Close the storage remote at cleanup time
This branch includes the following new commits:
new fa775a6 _testing/_fixtures.py: Added comment/hint around gRPC thread assertions
new 531254a sandbox.py: Make the sandbox a context manager and give it a _cleanup() method
new 1129965 element.py: Ensure that sandboxes are used within `with` statements
new 02f6a13 sandbox/_sandboxremote.py: Close the storage remote at cleanup time
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[buildstream] 02/04: sandbox.py: Make the sandbox a context manager and give it a _cleanup() method
Posted by tv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a commit to branch tristan/fix-leaking-grpc-threads
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 531254a908d9c20ba68ab04e5567deb1c22f0365
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Mon Feb 28 17:27:25 2022 +0900
sandbox.py: Make the sandbox a context manager and give it a _cleanup() method
Now sandbox classes can cleanup resources in their _cleanup() method when
the sandbox is discarded.
---
src/buildstream/sandbox/sandbox.py | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/src/buildstream/sandbox/sandbox.py b/src/buildstream/sandbox/sandbox.py
index 707951d..f309382 100644
--- a/src/buildstream/sandbox/sandbox.py
+++ b/src/buildstream/sandbox/sandbox.py
@@ -102,6 +102,20 @@ class Sandbox:
# Pending command batch
self.__batch = None
+ # __enter__()
+ #
+ # Called when entering the with-statement context.
+ #
+ def __enter__(self) -> "Sandbox":
+ return self
+
+ # __exit__()
+ #
+ # Called when exiting the with-statement context.
+ #
+ def __exit__(self, exc_type, exc_value, traceback) -> None:
+ self._cleanup()
+
def get_virtual_directory(self) -> Directory:
"""Fetches the sandbox root directory as a virtual Directory.
@@ -255,6 +269,13 @@ class Sandbox:
# Abstract Methods for Sandbox implementations #
#####################################################
+ # _cleanup():
+ #
+ # Abstract method to release resources when the sandbox is discarded
+ #
+ def _cleanup(self):
+ pass
+
# _run()
#
# Abstract method for running a single command
[buildstream] 03/04: element.py: Ensure that sandboxes are used within `with` statements
Posted by tv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a commit to branch tristan/fix-leaking-grpc-threads
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 1129965776313437c6237dfe21e5c59c4dc78d67
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Mon Feb 28 17:28:38 2022 +0900
element.py: Ensure that sandboxes are used within `with` statements
---
src/buildstream/element.py | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index e891349..419353c 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -2777,7 +2777,7 @@ class Element(Plugin):
self.info("Using a remote sandbox for artifact {} with directory '{}'".format(self.name, directory))
- sandbox = SandboxRemote(
+ with SandboxRemote(
context,
project,
directory,
@@ -2786,8 +2786,8 @@ class Element(Plugin):
stderr=stderr,
config=config,
output_node_properties=output_node_properties,
- )
- yield sandbox
+ ) as sandbox:
+ yield sandbox
elif directory is not None and os.path.exists(directory):
platform = context.platform
@@ -2803,7 +2803,8 @@ class Element(Plugin):
config=config,
output_node_properties=output_node_properties,
)
- yield sandbox
+ with sandbox:
+ yield sandbox
else:
os.makedirs(context.builddir, exist_ok=True)
[buildstream] 01/04: _testing/_fixtures.py: Added comment/hint around gRPC thread assertions
Posted by tv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a commit to branch tristan/fix-leaking-grpc-threads
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit fa775a604f6ef97b58bfe7747310880843d4743b
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Mon Feb 28 17:25:59 2022 +0900
_testing/_fixtures.py: Added comment/hint around gRPC thread assertions
When you find yourself painfully wondering what might be causing this
thread assertion to trigger, it can be useful to have a comment telling
you that it is probably due to BuildStream failing to close a gRPC channel.
---
src/buildstream/_testing/_fixtures.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/buildstream/_testing/_fixtures.py b/src/buildstream/_testing/_fixtures.py
index c2b8bb2..a4bba80 100644
--- a/src/buildstream/_testing/_fixtures.py
+++ b/src/buildstream/_testing/_fixtures.py
@@ -27,6 +27,10 @@ _AWAIT_THREADS_TIMEOUT_SECONDS = 5
def has_no_unexpected_background_threads(expected_num_threads):
# Use psutil as threading.active_count() doesn't include gRPC threads.
+ #
+ # If background gRPC threads are lingering, there is a good chance that
+ # this is due to BuildStream failing to close an open grpc channel.
+ #
process = psutil.Process()
wait = 0.1
[buildstream] 04/04: sandbox/_sandboxremote.py: Close the storage remote at cleanup time
Posted by tv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a commit to branch tristan/fix-leaking-grpc-threads
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 02f6a13de7a28d742302de09296701a687052d49
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Mon Feb 28 17:29:10 2022 +0900
sandbox/_sandboxremote.py: Close the storage remote at cleanup time
Fixes #1582
---
src/buildstream/sandbox/_sandboxremote.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/buildstream/sandbox/_sandboxremote.py b/src/buildstream/sandbox/_sandboxremote.py
index 2f2344e..4c0a86b 100644
--- a/src/buildstream/sandbox/_sandboxremote.py
+++ b/src/buildstream/sandbox/_sandboxremote.py
@@ -53,6 +53,7 @@ class SandboxRemote(SandboxREAPI):
self.operation_name = None
if self.storage_spec:
+ self.own_storage_remote = True
self.storage_remote = CASRemote(self.storage_spec, cascache)
try:
self.storage_remote.init()
@@ -61,6 +62,7 @@ class SandboxRemote(SandboxREAPI):
"Failed to contact remote execution CAS endpoint at {}: {}".format(self.storage_spec.url, e)
) from e
else:
+ self.own_storage_remote = False
self.storage_remote = cascache.get_default_remote()
def run_remote_command(self, channel, action_digest):
@@ -288,3 +290,7 @@ class SandboxRemote(SandboxREAPI):
raise SandboxError("Remote server failed at executing the build request.")
return execution_response.result
+
+ def _cleanup(self):
+ if self.own_storage_remote:
+ self.storage_remote.close()