Fix potential memory leak from DownloadController.once(canceled) assigned on every .download()

3 files changed, 23 insertions, 12 deletions

diff --git a/src/http-music.js b/src/http-music.js
index 3f1c901..12c93e7 100755
--- a/src/http-music.js
+++ b/src/http-music.js
@@ -33,6 +33,10 @@ function downloadPlaylistFromOptionValue(arg) {
   }
 }
 
+// Let this forever be of use to people who run into
+// maxlistenersexceededwarning.
+process.on('warning', e => console.warn(e.stack))
+
 Promise.resolve()
   .then(async () => {
     let sourcePlaylist = null
diff --git a/src/loop-play.js b/src/loop-play.js
index 701e590..a38e524 100644
--- a/src/loop-play.js
+++ b/src/loop-play.js
@@ -26,10 +26,16 @@ class DownloadController extends EventEmitter {
     // be canceled and replaced with a new download (see cancel)
     // which would void the result of the old download.)
 
+    this.cleanupListeners()
+
     let canceled = false
-    this.once('canceled', () => {
+
+    this._handleCanceled = () => {
       canceled = true
-    })
+      this.cleanupListeners()
+    }
+
+    this.once('canceled', this._handleCanceled)
 
     const file = await downloader(arg)
 
@@ -38,12 +44,19 @@ class DownloadController extends EventEmitter {
     }
   }
 
+  cleanupListeners() {
+    if (this._handleCanceled) {
+      this.removeListener('canceled', this._handleCanceled)
+    }
+  }
+
   cancel() {
     // Cancels the current download.  This doesn't cancel any
     // waitForDownload promises, though -- you'll need to start
     // a new download to resolve those.
 
     this.emit('canceled')
+    this.cleanupListeners()
   }
 }
 
@@ -128,7 +141,7 @@ class PlayController {
       file
     ])
 
-    const handleData = data => {
+    this.process.stderr.on('data', data => {
       const match = data.toString().match(
         /(..):(..):(..) \/ (..):(..):(..) \(([0-9]+)%\)/
       )
@@ -163,12 +176,6 @@ class PlayController {
           `\x1b[K~ (${percentStr}%) ${curStr} / ${lenStr}\r`
         )
       }
-    }
-
-    this.process.stderr.on('data', handleData)
-
-    this.process.once('exit', () => {
-      this.process.stderr.removeListener('data', handleData)
     })
 
     return new Promise(resolve => {
diff --git a/todo.txt b/todo.txt
index 6ad8e9f..6115430 100644
--- a/todo.txt
+++ b/todo.txt
@@ -173,10 +173,10 @@ TODO: Figure out the horrible, evil cause of the max listeners warning
       I'm getting lately.. current repro: play a bunch of files locally.
       Skipping tracks still leads to issue. Wait for them to start playing
       before skipping, though.
+      (Done!)
 
-TODO: Something's requesting avprobe, and I'm not sure why.
-      (While playing flora.json/YouTube videos.) Perhaps
-      it's youtube-dl..?
+TODO: Something's requesting avprobe, and I'm not sure why. (While playing
+      flora.json/YouTube videos.) Perhaps it's youtube-dl..?
 
 TODO: Re-implement skip.
       (Done!)