Memory leak fixes - stream and filter life cycles

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Add ability to export private keys as a file.
 - Add ability to export seed words as a file.
 - Changed state logs to a file download than a clipboard copy.
+- Fixed a long standing memory leak associated with filters installed by dapps
 - Fix link to support center.
 
 ## 3.10.0 2017-9-11

app/scripts/background.js

@@ -1,6 +1,8 @@
 const urlUtil = require('url')
 const endOfStream = require('end-of-stream')
 const pipe = require('pump')
+const log = require('loglevel')
+const extension = require('extensionizer')
 const LocalStorageStore = require('obs-store/lib/localStorage')
 const storeTransform = require('obs-store/lib/transform')
 const ExtensionPlatform = require('./platforms/extension')
@@ -9,13 +11,11 @@ const migrations = require('./migrations/')
 const PortStream = require('./lib/port-stream.js')
 const NotificationManager = require('./lib/notification-manager.js')
 const MetamaskController = require('./metamask-controller')
-const extension = require('extensionizer')
 const firstTimeState = require('./first-time-state')
 
 const STORAGE_KEY = 'metamask-config'
 const METAMASK_DEBUG = 'GULP_METAMASK_DEBUG'
 
-const log = require('loglevel')
 window.log = log
 log.setDefaultLevel(METAMASK_DEBUG ? 'debug' : 'warn')
 
@@ -29,12 +29,12 @@ let popupIsOpen = false
 const diskStore = new LocalStorageStore({ storageKey: STORAGE_KEY })
 
 // initialization flow
-initialize().catch(console.error)
+initialize().catch(log.error)
 
 async function initialize () {
   const initState = await loadStateFromPersistence()
   await setupController(initState)
-  console.log('MetaMask initialization complete.')
+  log.debug('MetaMask initialization complete.')
 }
 
 //

app/scripts/contentscript.js

@@ -1,11 +1,12 @@
+const fs = require('fs')
+const path = require('path')
+const pump = require('pump')
 const LocalMessageDuplexStream = require('post-message-stream')
 const PongStream = require('ping-pong-stream/pong')
-const PortStream = require('./lib/port-stream.js')
-const ObjectMultiplex = require('./lib/obj-multiplex')
+const ObjectMultiplex = require('obj-multiplex')
 const extension = require('extensionizer')
+const PortStream = require('./lib/port-stream.js')
 
-const fs = require('fs')
-const path = require('path')
 const inpageText = fs.readFileSync(path.join(__dirname, 'inpage.js')).toString()
 
 // Eventually this streaming injection could be replaced with:
@@ -50,22 +51,42 @@ function setupStreams () {
   pageStream.pipe(pluginStream).pipe(pageStream)
 
   // setup local multistream channels
-  const mx = ObjectMultiplex()
-  mx.on('error', console.error)
-  mx.pipe(pageStream).pipe(mx)
-  mx.pipe(pluginStream).pipe(mx)
+  const mux = new ObjectMultiplex()
+  pump(
+    mux,
+    pageStream,
+    mux,
+    (err) => logStreamDisconnectWarning('MetaMask Inpage', err)
+  )
+  pump(
+    mux,
+    pluginStream,
+    mux,
+    (err) => logStreamDisconnectWarning('MetaMask Background', err)
+  )
 
   // connect ping stream
   const pongStream = new PongStream({ objectMode: true })
-  pongStream.pipe(mx.createStream('pingpong')).pipe(pongStream)
+  pump(
+    mux,
+    pongStream,
+    mux,
+    (err) => logStreamDisconnectWarning('MetaMask PingPongStream', err)
+  )
 
   // connect phishing warning stream
-  const phishingStream = mx.createStream('phishing')
+  const phishingStream = mux.createStream('phishing')
   phishingStream.once('data', redirectToPhishingWarning)
 
   // ignore unused channels (handled by background, inpage)
-  mx.ignoreStream('provider')
-  mx.ignoreStream('publicConfig')
+  mux.ignoreStream('provider')
+  mux.ignoreStream('publicConfig')
+}
+
+function logStreamDisconnectWarning (remoteLabel, err) {
+  let warningMsg = `MetamaskContentscript - lost connection to ${remoteLabel}`
+  if (err) warningMsg += '\n' + err.stack
+  console.warn(warningMsg)
 }
 
 function shouldInjectWeb3 () {

app/scripts/lib/createLoggerMiddleware.js

@@ -0,0 +1,15 @@
+// log rpc activity
+module.exports = createLoggerMiddleware
+
+function createLoggerMiddleware({ origin }) {
+  return function loggerMiddleware (req, res, next, end) {
+    next((cb) => {
+      if (res.error) {
+        log.error('Error in RPC response:\n', res)
+      }
+      if (req.isMetamaskInternal) return
+      log.info(`RPC (${origin}):`, req, '->', res)
+      cb()
+    })
+  }
+}

app/scripts/lib/createOriginMiddleware.js

@@ -0,0 +1,9 @@
+// append dapp origin domain to request
+module.exports = createOriginMiddleware
+
+function createOriginMiddleware({ origin }) {
+  return function originMiddleware (req, res, next, end) {
+    req.origin = origin
+    next()
+  }
+}

app/scripts/lib/createProviderMiddleware.js

@@ -0,0 +1,13 @@
+
+module.exports = createProviderMiddleware
+
+// forward requests to provider
+function createProviderMiddleware({ provider }) {
+  return (req, res, next, end) => {
+    provider.sendAsync(req, (err, _res) => {
+      if (err) return end(err)
+      res.result = _res.result
+      end()
+    })
+  }
+}

app/scripts/lib/inpage-provider.js

@@ -1,73 +1,56 @@
-const pipe = require('pump')
-const StreamProvider = require('web3-stream-provider')
+const pump = require('pump')
+const RpcEngine = require('json-rpc-engine')
+const createIdRemapMiddleware = require('json-rpc-engine/src/idRemapMiddleware')
+const createStreamMiddleware = require('json-rpc-middleware-stream')
 const LocalStorageStore = require('obs-store')
-const ObjectMultiplex = require('./obj-multiplex')
-const createRandomId = require('./random-id')
+const ObjectMultiplex = require('obj-multiplex')
 
 module.exports = MetamaskInpageProvider
 
 function MetamaskInpageProvider (connectionStream) {
   const self = this
 
   // setup connectionStream multiplexing
-  var multiStream = self.multiStream = ObjectMultiplex()
-  pipe(
+  const mux = self.mux = new ObjectMultiplex()
+  pump(
     connectionStream,
-    multiStream,
+    mux,
     connectionStream,
     (err) => logStreamDisconnectWarning('MetaMask', err)
   )
 
   // subscribe to metamask public config (one-way)
   self.publicConfigStore = new LocalStorageStore({ storageKey: 'MetaMask-Config' })
-  pipe(
-    multiStream.createStream('publicConfig'),
+  pump(
+    mux.createStream('publicConfig'),
     self.publicConfigStore,
     (err) => logStreamDisconnectWarning('MetaMask PublicConfigStore', err)
   )
 
   // ignore phishing warning message (handled elsewhere)
-  multiStream.ignoreStream('phishing')
+  mux.ignoreStream('phishing')
 
   // connect to async provider
-  const asyncProvider = self.asyncProvider = new StreamProvider()
-  pipe(
-    asyncProvider,
-    multiStream.createStream('provider'),
-    asyncProvider,
+  const streamMiddleware = createStreamMiddleware()
+  pump(
+    streamMiddleware.stream,
+    mux.createStream('provider'),
+    streamMiddleware.stream,
     (err) => logStreamDisconnectWarning('MetaMask RpcProvider', err)
   )
-  // start and stop polling to unblock first block lock
 
-  self.idMap = {}
+  // handle sendAsync requests via dapp-side rpc engine
+  const rpcEngine = new RpcEngine()
+  rpcEngine.push(createIdRemapMiddleware())
+  rpcEngine.push(streamMiddleware)
+  self.rpcEngine = rpcEngine
 }
 
 // handle sendAsync requests via asyncProvider
 // also remap ids inbound and outbound
 MetamaskInpageProvider.prototype.sendAsync = function (payload, cb) {
   const self = this
-
-  // rewrite request ids
-  const request = eachJsonMessage(payload, (_message) => {
-    const message = Object.assign({}, _message)
-    const newId = createRandomId()
-    self.idMap[newId] = message.id
-    message.id = newId
-    return message
-  })
-
-  // forward to asyncProvider
-  self.asyncProvider.sendAsync(request, (err, _res) => {
-    if (err) return cb(err)
-    // transform messages to original ids
-    const res = eachJsonMessage(_res, (message) => {
-      const oldId = self.idMap[message.id]
-      delete self.idMap[message.id]
-      message.id = oldId
-      return message
-    })
-    cb(null, res)
-  })
+  self.rpcEngine.handle(payload, cb)
 }
 
 
@@ -124,14 +107,6 @@ MetamaskInpageProvider.prototype.isMetaMask = true
 
 // util
 
-function eachJsonMessage (payload, transformFn) {
-  if (Array.isArray(payload)) {
-    return payload.map(transformFn)
-  } else {
-    return transformFn(payload)
-  }
-}
-
 function logStreamDisconnectWarning (remoteLabel, err) {
   let warningMsg = `MetamaskInpageProvider - lost connection to ${remoteLabel}`
   if (err) warningMsg += '\n' + err.stack

app/scripts/lib/port-stream.js

@@ -1,5 +1,6 @@
 const Duplex = require('readable-stream').Duplex
 const inherits = require('util').inherits
+const noop = function(){}
 
 module.exports = PortDuplexStream
 
@@ -20,20 +21,14 @@ PortDuplexStream.prototype._onMessage = function (msg) {
   if (Buffer.isBuffer(msg)) {
     delete msg._isBuffer
     var data = new Buffer(msg)
-    // console.log('PortDuplexStream - saw message as buffer', data)
     this.push(data)
   } else {
-    // console.log('PortDuplexStream - saw message', msg)
     this.push(msg)
   }
 }
 
 PortDuplexStream.prototype._onDisconnect = function () {
-  try {
-    this.push(null)
-  } catch (err) {
-    this.emit('error', err)
-  }
+  this.destroy()
 }
 
 // stream plumbing
@@ -45,19 +40,12 @@ PortDuplexStream.prototype._write = function (msg, encoding, cb) {
     if (Buffer.isBuffer(msg)) {
       var data = msg.toJSON()
       data._isBuffer = true
-      // console.log('PortDuplexStream - sent message as buffer', data)
       this._port.postMessage(data)
     } else {
-      // console.log('PortDuplexStream - sent message', msg)
       this._port.postMessage(msg)
     }
   } catch (err) {
-    // console.error(err)
     return cb(new Error('PortDuplexStream - disconnected'))
   }
   cb()
 }
-
-// util
-
-function noop () {}

app/scripts/lib/stream-utils.js

@@ -1,6 +1,6 @@
 const Through = require('through2')
-const endOfStream = require('end-of-stream')
-const ObjectMultiplex = require('./obj-multiplex')
+const ObjectMultiplex = require('obj-multiplex')
+const pump = require('pump')
 
 module.exports = {
   jsonParseStream: jsonParseStream,
@@ -23,14 +23,14 @@ function jsonStringifyStream () {
 }
 
 function setupMultiplex (connectionStream) {
-  var mx = ObjectMultiplex()
-  connectionStream.pipe(mx).pipe(connectionStream)
-  endOfStream(mx, function (err) {
-    if (err) console.error(err)
-  })
-  endOfStream(connectionStream, function (err) {
-    if (err) console.error(err)
-    mx.destroy()
-  })
-  return mx
+  const mux = new ObjectMultiplex()
+  pump(
+    connectionStream,
+    mux,
+    connectionStream,
+    (err) => {
+      if (err) console.error(err)
+    }
+  )
+  return mux
 }