Socket.io memory leak when used with HTTPS/TLS server and Node 0.8.x

[jpallen]

We recently tried to upgrade our app to Node 0.8 from 0.6 and ran into a large
memory leak which we've traced back to socket.io. The fault really lies with how
Node 0.8 handles buffers in its TLS library, but Socket.io triggers the problem.

The problem is that Socket.io holds a reference to the head buffer which comes
in on the upgrade event from the server (line 617 of lib/manager.js). This
reference is held for the duration of the lifetime of the WebSocket connection.
This is a small buffer and so is not in itself responsible for the memory leak.
The trouble is that in Node.js 0.8, the TLS library allocates large
10Mb buffers which it thens allocates portions of to smaller buffers for incoming
requests. The head buffer passed to the upgrade event is actually part of a
larger 10Mb buffer. These 10Mb buffers are not freed by the garbage collector if
any of their child buffers are still in use, and so holding a reference to the
small head buffer is actually keeping a much larger 10Mb buffer from
being freed when socket.io is used with the HTTPS server.

Some Data

I've created a small test script which can found in the
https://github.com/jpallen/socket.io-memory-test repository (also included below).
This starts a
simple HTTPS and WebSocket server that listens for incoming
connections but does nothing with them. Every second a new socket.io client
connects to the WebSocket server, and every quarter of a second a 265Kb image
is uploaded to the HTTPS server. The image upload ensures that plenty of
10Mb buffers are created, and the continual stream of WebSocket connections
ensures that there are plenty of references to the req.head buffer which hold on
to these 10Mb buffers. The test script can be run with only uploads, only
websockets or both. The results are quite clear:

Neither WebSocket connections nor the uploads are a problem themselves and allow
the garbage collector to do its job. Together however there is a significant
memory leak.

Fix

The fix is simple. req.head is only used by the websockets/default.js
transport and it is safe to discard it once this has established the connection.
We can do this almost immediately in lib/manager.js:

req.head = head;
this.handleClient(data, req);
req.head = null

With this patch applied, the combination of WebSockets and an HTTPS behaves how
one would expect:

The memory usage with WebSockets and uploads together now looks like the
supposition of the two individually. (The scale is much
smaller on this graph compared to the leaking example above.)

Remarks

I'm going to create an issue for Node.js about this as well. I think this is a design
flaw in the way Node 0.8 handles buffers in the TLS library since holding on to a
small buffer like head for an extended period of time should not have such a dramatic
effect for the rest of the app.

The TLS server in Node 0.6 doesn't allocate buffers in large 10Mb chunks and so
is unaffected by this memory leak. Here is an example run for Node 0.6 if you're
interested:

The graph is less linear because we don't have control over garbage collecting.
Note again the smaller scale compared to the leaking example.

Test Script

You need an file called image.png in the same directory as this script.

# test.coffee
socketio = require "socket.io"
https = require "https"
client = require "socket.io-client"
request = require "request"
fs = require "fs"
argv = require("optimist")
    .options "s"
        alias: "websockets"
        describe: "Client connects to server with websockets"
        default: true
    .options "u"
        alias: "uploads"
        describe: "Client sends large POST requests to server"
        default: true
    .argv

options =
    key: fs.readFileSync('server.key')
    cert: fs.readFileSync('server.crt')

server = https.createServer(options).listen(5000)
io = socketio.listen(server, "log level": 0)

server.on "request", (req, res) ->
    req.on "data", (chunk) ->
    req.on "end", () -> res.end()

fs = require "fs"

# Client stuff
if argv["websockets"]
    setInterval (
        () ->
            client.connect("https://localhost:5000", "force new connection" : true)
    ), 1000

if argv["uploads"]
    setInterval (
        () ->
            fs.createReadStream("image.png").pipe(request.post("https://localhost:5000/upload"))
    ), 250

# Garbage collect regularly for consistency
# Run node with --expose-gc for this to be available
if gc?
    setInterval (
        () -> gc()
    ), 1000

# Log memory usage
setInterval (
    () ->
        memory = process.memoryUsage()
        console.log [
            memory.rss
            memory.heapUsed
            memory.heapTotal
        ].join(",")
), 1000

# Run for two minutes
setTimeout (() -> process.exit()), 2 * 60 * 1000

Usage

coffee --nodejs "--expose-gc" test.coffee [--no-uploads] [--no-websockets]

A more complete test suite can found in jpallen/socket.io-memory-test from my
various experiments.

[jacobbeasleyfn]

Has this been fixed in node 0.9.22?

[jpallen]

Do you mean 0.8.22? I highly doubt it since it would require a big change in the way the tls module handles buffers internally. I don't think this aspect of Node has changed in 0.10.x either.

[jeff-minard-ck]

It has not. I've got a production nodejs instances (0.10.1) restarting nightly due to memory leak of ssl+socket.io. I suspect it's related to all this.

[jacobbeasleyfn]

We got this to work well, finally. I think this change worked after all!

Jake

On Fri, Mar 29, 2013 at 3:53 PM, Jeff Minard [email protected]:

It has not. I've got a production nodejs instances restarting nightly due
to memory leak of ssl+socket.io. I suspect it's related to all this.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1177#issuecomment-15660148
.

Jacob Beasley
Field Nation
612-210-7533

[rauchg]

0.9.14 released with this patch. Thanks @jpallen

[arianitu]

Do we know what node versions this applies to? Does this apply to v0.10.22?