seigel / pouchdb-react-native

Pouchdb with async storage
MIT License
483 stars 70 forks source link

setMaxListeners seems to have no effect #33

Closed cmrichards closed 7 years ago

cmrichards commented 8 years ago

Hey,

Thanks for the great library!

I keep getting this message in the Android emulator :+1:

console.error "(node) warning: possible EventEmitter memory leak deteced. %d listeners added. Use emitter.setMaxListeners() to increase limit.", 11

According to the official docs (https://pouchdb.com/errors.html#event_emitter_limit) the answer is to do this : db.setMaxListeners(20);

I've tried this but I still keep getting the message.

I'm fairly confident that I call cancel() on any changes(), replicate(), or sync() requests, so I'm not really sure what's causing it.

Any ideas?

Thanks! Chris

stockulus commented 8 years ago

Hi Chris,

my only Idee is to set additional require('events').EventEmitter.defaultMaxListeners(20) Otherwise I'd like you to ask for some sample Code that reproduced the Error?

Christoph

cmrichards commented 8 years ago

I added :

import events from "events"
events.EventEmitter.defaultMaxListeners = 20

And it did work, but my app still eventually hits the new limit of 21.

I'm pretty certain that the issue is that one of my change listeners is not being cancelled when I call cancel() on it. I have tested this by opening the same Sample (see code below) component a specific number of times until I see the error. This error only seems to happen on the android emulator and not on my device when I create an apk.

If I call this.listener.cancel() immediately after I create the listener in componentDidMount() then I don't see the error.

class Samples extends React.Component {

  constructor(props) {
    super(props)
    this.state = {
      samples: []
    }
  }

  componentWillUnmount() {
    this.listener.cancel();
    console.log("SAMPLES BEING UNMOUNTED")
  }

  componentDidMount() {
    const { job, database } = this.props;
    this.listener  = database.fetchAndListenForSampleChanges(job, this.updateSamples)
  }
  ...

}

// This is the database props above
// There is only one instance of this Database class which is created at (or near) the root component 
// of the app and then passed down to anything that needs it
export default class Database {
  constructor() {
    this.localDB = new PouchDB('jobs');
    this.remoteDB = new PouchDB('https://....cloudant.com/....', {
                                 auth: {
                                   username: 'xxx',
                                   password: 'xxxx'
                                 }
                              });
    this.sync = this.localDB.sync(this.remoteDB, {
      live: true,
      retry: true
    })
  }

  // This is only called in componentWillUnmount of the root app component
  closeConnection() {
    this.sync.cancel()
    this.localDB.close()
    this.remoteDB.close()
    this.sync = null
    this.localDB = null
    this.remoteDB = null
  }

  fetchAndListenForSampleChanges(job, callback) {
    this.fetchSamplesForJob(job, callback);
    var listener = this.localDB.changes({
      live: true,
      since: "now",
      filter: (doc) => doc.type === "sample"
    }).on('change', callback)
    return listener;
  }
  ...
}
stockulus commented 8 years ago

hmmm sound difficult... and fits in my theory, Android Development on the Device ;)... but this does not really help.

How often in you app do you call fetchAndListenForSampleChanges?

let counter = 0
fetchAndListenForSampleChanges(job, callback) {
    counter ++
    console.log('Counter++' counter)
    this.fetchSamplesForJob(job, callback);
    var listener = this.localDB.changes({
      live: true,
      since: "now",
      filter: (doc) => doc.type === "sample"
    }).on('change', callback)
    return {
       cancel: () => {
          listener.cancel()
          counter--
          console.log('Counter--' counter)
       }
    }
  }

can just run this and check if the output is the expected?

cmrichards commented 8 years ago

fetchAndListenForSampleChanges is only called on one page.

I applied your code, and then went back and forth between the page and the previous one until I got this error :

I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
I/ReactNativeJS( 2748): Counter--0
I/ReactNativeJS( 2748): Counter++1
E/ReactNativeJS( 2748): '(node) warning: possible EventEmitter memory leak detected. %d listeners added. Use emitter.setMaxListeners() to increase limit.', 21

The other page I went back to only creates one "changes" listener and this page is not unmounted and re-mounted because it is part of the navigation stack (NavigationExperimenta) and in the history, so that page isn'tthe issue. The call to cancel() musn't be working.

stockulus commented 8 years ago

Ok, that proves your code is fine.

Now the big Question is, where is the bug. In the "adapter-asyncstorage" or in PouchDB Core. I'll check it, but have to find some time for it

cmrichards commented 8 years ago

Fortunately, it doesn't break on a device. Although maybe that explains why the DB reads seem to get slower as I use the app?

Here's all the code if you're curious : https://github.com/cmrichards/rn-jobs-and-samples-pouchdb-test

stockulus commented 7 years ago

reopen, if this is still a issue

ghost commented 7 years ago

Hello,

I can also confirm this issue, I already set the setMaxListeners to 100, but the problem is that onChange-updates are still coming in even if the Component is unmounted, and I cancel the synchronization in componentWillUnmount(), which leads to wrong this.setState({ . . . }) calls since setState() requires a mounted component.

Any idea? And did someone find out in the meantime in which package the addEventListener() (is it?) resides?