From 2c66335a9b63387728496e2fea4fbefc080ceebe Mon Sep 17 00:00:00 2001
From: Peter Urbanec <git.user@urbanec.net>
Date: Sun, 31 Jan 2021 07:01:28 +1100
Subject: [PATCH] upnpevents: Fix leaked sockets

Commit f9a78d598e48132a8c6cf9ce31b51163b6bd2f67 refactored the code, but
in the process introduced a socket leak. When connect() is called on a
socket set to non-blocking mode, the returned error code is EINPROGRESS.
In that case, the code never initialises the ev structure and the socket
reference is lost. Given enough time (on my network about a day and a half)
this will eventually lead to a non-responsive server because the process
runs out of fds. Netstat will show an excessive number of sockets stuck in
CLOSE_WAIT state forever and ls /proc/<pid>/fd will confirm a lot of open
sockets.

Initialising the ev struct before a call to connect() ensures that the
socket reference is not lost. Verifying with netstat, one can see the
sockets in TIME_WAIT state for a brief period of time and /proc/<pid>/fd
shows a reasonable number of sockets being open.
---
 upnpevents.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/upnpevents.c b/upnpevents.c
index 4de6ce8..057066c 100644
--- a/upnpevents.c
+++ b/upnpevents.c
@@ -239,23 +239,23 @@ upnp_event_create_notify(struct subscriber *sub)
 
 	obj = calloc(1, sizeof(struct upnp_event_notify));
 	if(!obj) {
-		DPRINTF(E_ERROR, L_HTTP, "%s: calloc(): %s\n", "upnp_event_create_notify", strerror(errno));
+		DPRINTF(E_ERROR, L_HTTP, "calloc(): %s\n", strerror(errno));
 		return;
 	}
 	obj->sub = sub;
 	s = socket(PF_INET, SOCK_STREAM, 0);
 	if(s < 0) {
-		DPRINTF(E_ERROR, L_HTTP, "%s: socket(): %s\n", "upnp_event_create_notify", strerror(errno));
+		DPRINTF(E_ERROR, L_HTTP, "socket(): %s\n", strerror(errno));
 		goto error;
 	}
 	if((flags = fcntl(s, F_GETFL, 0)) < 0) {
-		DPRINTF(E_ERROR, L_HTTP, "%s: fcntl(..F_GETFL..): %s\n",
-		       "upnp_event_create_notify", strerror(errno));
+		DPRINTF(E_ERROR, L_HTTP, "fcntl(..F_GETFL..): %s\n",
+		       strerror(errno));
 		goto error;
 	}
 	if(fcntl(s, F_SETFL, flags | O_NONBLOCK) < 0) {
-		DPRINTF(E_ERROR, L_HTTP, "%s: fcntl(..F_SETFL..): %s\n",
-		       "upnp_event_create_notify", strerror(errno));
+		DPRINTF(E_ERROR, L_HTTP, "fcntl(..F_SETFL..): %s\n",
+		       strerror(errno));
 		goto error;
 	}
 	if(sub)
@@ -290,18 +290,18 @@ upnp_event_create_notify(struct subscriber *sub)
 	addr.sin_family = AF_INET;
 	inet_aton(obj->addrstr, &addr.sin_addr);
 	addr.sin_port = htons(port);
-	DPRINTF(E_DEBUG, L_HTTP, "%s: '%s' %hu '%s'\n", "upnp_event_notify_connect",
+	DPRINTF(E_DEBUG, L_HTTP, "'%s' %hu '%s'\n",
 	       obj->addrstr, port, obj->path);
 	obj->state = EConnecting;
+	obj->ev = (struct event ){ .fd = s, .rdwr = EVENT_WRITE,
+		.process = upnp_event_process_notify, .data = obj };
+	event_module.add(&obj->ev);
 	if(connect(s, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
 		if(errno != EINPROGRESS && errno != EWOULDBLOCK) {
-			DPRINTF(E_ERROR, L_HTTP, "%s: connect(): %s\n", "upnp_event_notify_connect", strerror(errno));
+			DPRINTF(E_ERROR, L_HTTP, "connect(): %s\n", strerror(errno));
 			obj->state = EError;
+			event_module.del(&obj->ev, 0);
 		}
-	} else {
-		obj->ev = (struct event ){ .fd = s, .rdwr = EVENT_WRITE,
-		    .process = upnp_event_process_notify, .data = obj };
-		event_module.add(&obj->ev);
 	}
 
 	return;