From 3d9f47be1d315de3e2edfe789b2503a6cffe02a6 Mon Sep 17 00:00:00 2001
From: Tobias Henkel <tobias.henkel@bmw.de>
Date: Sun, 3 Feb 2019 13:25:01 +0100
Subject: [PATCH] Properly close streams

When running test cases in the debugger it spits out resource warnings
about non-closed streams [1]. Explicitly closing the streams of the
subprocesses and log streamer fixes this warning. Maybe this even
solves the memory leak we're currently seeing in the executors.

[1] Trace:
/zuul/executor/server.py:1894: ResourceWarning: unclosed file <_io.BufferedReader name=60>
  self.proc = None
ResourceWarning: Enable tracemalloc to get the object allocation traceback

/zuul/executor/server.py:124: ResourceWarning: unclosed file <_io.BufferedReader name=11>
  stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
ResourceWarning: Enable tracemalloc to get the object allocation traceback

Change-Id: I65f191dc2e50f9c84f5bf6a3967d768d7ebe6b04
---
 tests/unit/test_streaming.py | 1 +
 zuul/executor/server.py      | 2 ++
 zuul/lib/log_streamer.py     | 5 +++++
 zuul/web/__init__.py         | 2 ++
 4 files changed, 10 insertions(+)

diff --git a/tests/unit/test_streaming.py b/tests/unit/test_streaming.py
index 2345aedddc..35fbcfa46a 100644
--- a/tests/unit/test_streaming.py
+++ b/tests/unit/test_streaming.py
@@ -55,6 +55,7 @@ class WSClient(WebSocketBaseClient):
         self.send(json.dumps(req))
         self.event.set()
         super(WSClient, self).run()
+        self.close()
 
 
 class TestLogStreamer(tests.base.BaseTestCase):
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 219c2342b6..7b4dfe227f 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -141,6 +141,7 @@ class DiskAccountant(object):
                         .format(size=size, job=dirname, limit=self.limit))
                     self.usage_func(dirname, size)
             du.wait()
+            du.stdout.close()
             after = time.time()
             # Sleep half as long as that took, or 1s, whichever is longer
             delay_time = max((after - before) / 2, 1.0)
@@ -1891,6 +1892,7 @@ class AnsibleJob(object):
             self.log.debug("Stopped disk job killer")
 
         with self.proc_lock:
+            self.proc.stdout.close()
             self.proc = None
 
         if timeout and watchdog.timed_out:
diff --git a/zuul/lib/log_streamer.py b/zuul/lib/log_streamer.py
index 1eed621534..16b7222789 100644
--- a/zuul/lib/log_streamer.py
+++ b/zuul/lib/log_streamer.py
@@ -98,6 +98,11 @@ class RequestHandler(streamer_utils.BaseFingerRequestHandler):
                 if self.follow_log(log):
                     break
                 else:
+                    if log is not None:
+                        try:
+                            log.file.close()
+                        except Exception:
+                            pass
                     return
 
     def chunk_log(self, log_file):
diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py
index dbbb707390..3acf4807a2 100755
--- a/zuul/web/__init__.py
+++ b/zuul/web/__init__.py
@@ -185,9 +185,11 @@ class LogStreamer(object):
                 if data:
                     self.websocket.send(data, False)
                 self.zuulweb.stream_manager.unregisterStreamer(self)
+                self.finger_socket.close()
                 return self.websocket.logClose(1000, "No more data")
         else:
             self.zuulweb.stream_manager.unregisterStreamer(self)
+            self.finger_socket.close()
             return self.websocket.logClose(1000, "Remote error")