From 8e5e49984d5017778549e8f9570a82f609305da3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 17 Sep 2020 12:50:02 -0700
Subject: [PATCH] Fix a slow memory leak in long-lived FutureIterator objects,
 as used by FuturePool

Summary:
See T13572. FutureIterator does not release futures, so long-lived iterators (like the one that FuturePool may build) can end up leaking memory.

This affects the FuturePool used by the daemon overseer.

See T13572 for more discussion.

Test Plan:
  - Ran the simple FutureIterator script from T13572. Before: memory held during iteration, script grows without bound. After: memory released, script uses stable memory.
  - Ran the overseer with memory logging and an immediate wakeup from hibernation. Before: saw memory usage grow without bound at a rate of ~300MB/day. After: saw memory usage stable.

Differential Revision: https://secure.phabricator.com/D21466
---
 src/future/FutureIterator.php | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/future/FutureIterator.php b/src/future/FutureIterator.php
index 028c02da..96a41e3d 100644
--- a/src/future/FutureIterator.php
+++ b/src/future/FutureIterator.php
@@ -194,7 +194,14 @@ final class FutureIterator
    * @task iterator
    */
   public function next() {
-    $this->key = null;
+    // See T13572. If we preivously resolved and returned a Future, release
+    // it now. This prevents us from holding Futures indefinitely when callers
+    // like FuturePool build long-lived iterators and keep adding new Futures
+    // to them.
+    if ($this->key !== null) {
+      unset($this->futures[$this->key]);
+      $this->key = null;
+    }
 
     $this->updateWorkingSet();