Fix socket 'connect' memory leak (#465)

Nodge · sindresorhus · commit 13d6b68ba024 · 2018-03-04T21:47:11.000+07:00
diff --git a/index.js b/index.js
@@ -227,14 +227,18 @@ function requestAsEventEmitter(opts) {
 					total: uploadBodySize
 				});
 
-				if (req.connection) {
-					req.connection.once('connect', () => {
+				const socket = req.connection;
+				if (socket) {
+					// `._connecting` was the old property which was made public in node v6.1.0
+					const isConnecting = socket.connecting === undefined ? socket._connecting : socket.connecting;
+
+					const onSocketConnect = () => {
 						const uploadEventFrequency = 150;
 
 						progressInterval = setInterval(() => {
 							const lastUploaded = uploaded;
 							const headersSize = Buffer.byteLength(req._header);
-							uploaded = req.connection.bytesWritten - headersSize;
+							uploaded = socket.bytesWritten - headersSize;
 
 							// Prevent the known issue of `bytesWritten` being larger than body size
 							if (uploadBodySize && uploaded > uploadBodySize) {
@@ -254,7 +258,17 @@ function requestAsEventEmitter(opts) {
 								total: uploadBodySize
 							});
 						}, uploadEventFrequency);
-					});
+					};
+
+					// Only subscribe to 'connect' event if we're actually connecting a new
+					// socket, otherwise if we're already connected (because this is a
+					// keep-alive connection) do not bother. This is important since we won't
+					// get a 'connect' event for an already connected socket.
+					if (isConnecting) {
+						socket.once('connect', onSocketConnect);
+					} else {
+						onSocketConnect();
+					}
 				}
 			});
 
diff --git a/test/agent.js b/test/agent.js
@@ -134,10 +134,14 @@ test('redirects from https to http work with an agent object', async t => {
 
 test('socket connect listener cleaned up after request', async t => {
 	const {agent} = createAgentSpy(HttpsAgent);
-	await got(`${https.url}`, {
-		rejectUnauthorized: false,
-		agent
-	});
+	// Make sure there are no memory leaks when reusing keep-alive sockets
+	for (let i = 0; i < 20; i++) {
+		// eslint-disable-next-line no-await-in-loop
+		await got(`${https.url}`, {
+			rejectUnauthorized: false,
+			agent
+		});
+	}
 	Object.keys(agent.freeSockets).forEach(k => agent.freeSockets[k]
 		.forEach(sock => t.is(sock.listenerCount('connect'), 0)));
 	// Make sure to close all open sockets
