danmactough / node-feedparser

Robust RSS, Atom, and RDF feed parsing in Node.js
Other
1.97k stars 192 forks source link

Memory leak error when parsing a large feed #169

Closed jetlej closed 8 years ago

jetlej commented 8 years ago

From what I've read around the web, I need to remove the listeners on req and feedparser, otherwise I'll get a memory leak. Not sure how to do that though... Right now this function is taking down a 4GB server whenever it runs.

Btw this is being run in Meteor.

parseFeed: function(url) {
    var req = request(url),
        feedparser = new FeedParser([]),
        count = 0,
        limit = 15;

    req.on('error', function (error) {
        console.log(error);
    });

    req.on('response', function (response) {
        var stream = this;

        if (response.statusCode != 200) {
            return;
        }
        stream.pipe(feedparser);
    });

    feedparser.on('error', function (error) {
        console.log(error);
    });

    feedparser.on('readable', function() {

        // This is where the action is!
        var stream = this,
        meta = this.meta, 
        item;

        while (item = stream.read()) {
            Fiber(function() {
                if (item && count < limit) {
                    try {
                        function __call(item, meta) {
                               saveArticle(item);
                        }

                        __call(item, meta);

                    } catch (e) {
                        console.log(e);
                    }
                    count++;
                }
            }).run();
        }
    });
},
danmactough commented 8 years ago

See #124

I don't know anything about Meteor or Fiber. Maybe when you close over the item or meta with your __call function, something about Fiber causes v8 to be unable to know when it can garbage collect those objects.