« get me outta code hell

Fix potential memory leak from DownloadController.once(canceled) assigned on every .download() - http-music - Command-line music player + utils (not a server!)
about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authorLiam <towerofnix@gmail.com>2017-07-16 09:16:31 -0400
committerLiam <towerofnix@gmail.com>2017-07-16 09:16:31 -0400
commitebada93f0d2b2ec3a158dae2ba43e98d81f3182a (patch)
treea6e984aad71184cc673db1198f874d3a42b0e5b6 /src
parent02059af9feea6326a76be2f5cf623d6ca86a4f78 (diff)
Fix potential memory leak from DownloadController.once(canceled) assigned on every .download()
Diffstat (limited to 'src')
-rwxr-xr-xsrc/http-music.js4
-rw-r--r--src/loop-play.js25
2 files changed, 20 insertions, 9 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 => {