Search code examples
javanode.jspush-notificationlong-polling

Node.js: Does this long polling implementation leak:


I'm evaluating Node.js for possible replacement of my current push functionality on a Java Web App. I wrote a simple long polling server that works like an intermediary between the client and the Java back-end. The client makes a request to subscribe, and then the Java server can notify subscribed clients by calling Node.js. It seems to be working fine so far, but I got the following message which points to a memory leak:

    (node) warning: possible EventEmitter memory leak detected. 11 listeners added.
Use emitter.setMaxListeners() to increase limit.
Trace
    at EventEmitter.addListener (events.js:168:15)
    at EventEmitter.once (events.js:189:8)
    at route (C:\Users\Juan Pablo\pushserver.js:42:12)
    at Server.onRequest (C:\Users\Juan Pablo\pushserver.js:32:3)
    at Server.EventEmitter.emit (events.js:91:17)
    at HTTPParser.parser.onIncoming (http.js:1793:12)
    at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:111:23
)
    at Socket.socket.ondata (http.js:1690:22)
    at TCP.onread (net.js:402:27)

I have a line of code that logs the existing listeners whenever a notify event is emitted. I've had it running for a while and it shows that there is only one listener per subscribed client (as should be), but this line wasn't on the code when I got the warning message. The code was exactly the same except for that line tough.

This is the push server's code (it's a bit rudimentary since I'm still learning Node.js):

var http = require('http');
var url = require("url");
var qs = require("querystring");
var events = require('events');
var util = require('util');

var emitter = new events.EventEmitter;

function onRequest(request, response)
{
    var pathname = url.parse(request.url).pathname;
    console.log("Request for " + pathname + " received.");
    request.setEncoding("utf8");

    if (request.method == 'POST')
    {
        var postData = "";
        request.addListener("data", function(postDataChunk)
        {
            postData += postDataChunk;
            console.log("Received POST data chunk '"+ postDataChunk + "'.");
        });

        request.addListener("end", function()
        {
            route(pathname, response, postData);
        });
    }
    else if (request.method=='GET')
    {
        var urlParts = url.parse(request.url, true);
        route(pathname, response, urlParts.query);
    }
}

function route(pathname, response, data)
{
    switch (pathname)
    {
        case "/subscription":

            emitter.once("event:notify", function(ids)
            {
                response.writeHead(200, {"Content-Type": "text/html", "Access-Control-Allow-Origin": "*"});
                response.write(JSON.stringify(ids));
                response.end();
            });

            break;

        case "/notification":

            //show how many listeners exist
            console.log(util.inspect(emitter.listeners('event:notify'));

            emitter.emit("event:notify", data.ids);

            response.writeHead(200, {"Content-Type": "text/html", "Access-Control-Allow-Origin": "*"});
            response.write(JSON.stringify(true));
            response.end();
            break;

        default:

            console.log("No request handler found for " + pathname);
            response.writeHead(404, {"Content-Type": "text/plain", "Access-Control-Allow-Origin": "*"});
            response.write("404 - Not found");
            response.end();
            break;
    }
}

http.createServer(onRequest).listen(8888);

console.log('Server running at http://127.0.0.1:8888/');

I was under the impression that using emitter.once would automatically remove the event listener once it was used, so I don't know how 11 listeners could've been added if there was only one client connected. I'm thinking that perhaps if the client disconnects while waiting for a notification then the associated connection resources are not disposed.

I'm wondering whether I have to manually handle disconnections and if there is actually a leak in there somewhere. Any advice is welcome. Thanks.


Solution

  • If anyone is interested, the above code does leak. The leak occurs when a client disconnects before a notification is sent. To fix this, it is necessary to remove the event listener when a client disconnects abruptly, such as:

    case "/subscription":
    
            var notify = function(ids)
            {
                response.writeHead(200, {"Content-Type": "text/html", "Access-Control-Allow-Origin": "*"});
                response.write(JSON.stringify(ids));
                response.end();
            }
    
            emitter.once("event:notify", notify);
    
            //event will be removed when connection is closed
            request.on("close", function()
            {
                emitter.removeListener("event:notify", notify);
            });
            break;