From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Thu, 31 Mar 2016 22:07:59 +0000 (+0200)
Subject: Delete streams when they are closed (prevents memory leak on disconnecting clients).
X-Git-Tag: 1.2.0~39
X-Git-Url: https://git.sesse.net/?p=nageru;a=commitdiff_plain;h=dbadc60a477927db7a5e941b17fe1c9b808fd175

Delete streams when they are closed (prevents memory leak on disconnecting clients).
---

diff --git a/httpd.cpp b/httpd.cpp
index 4b24890..b9b1d6f 100644
--- a/httpd.cpp
+++ b/httpd.cpp
@@ -14,6 +14,8 @@ extern "C" {
 #include <libavutil/samplefmt.h>
 }
 
+#include <vector>
+
 #include "httpd.h"
 
 #include "defs.h"
@@ -34,11 +36,14 @@ void HTTPD::start(int port)
 	MHD_start_daemon(MHD_USE_THREAD_PER_CONNECTION | MHD_USE_POLL_INTERNALLY | MHD_USE_DUAL_STACK,
 	                 port,
 			 nullptr, nullptr,
-			 &answer_to_connection_thunk, this, MHD_OPTION_END);
+			 &answer_to_connection_thunk, this,
+	                 MHD_OPTION_NOTIFY_COMPLETED, &request_completed_thunk, this, 
+	                 MHD_OPTION_END);
 }
 
 void HTTPD::add_packet(const AVPacket &pkt, int64_t pts, int64_t dts)
 {
+	unique_lock<mutex> lock(streams_mutex);
 	for (Stream *stream : streams) {
 		stream->add_packet(pkt, pts, dts);
 	}
@@ -84,11 +89,14 @@ int HTTPD::answer_to_connection(MHD_Connection *connection,
 				const char *version, const char *upload_data,
 				size_t *upload_data_size, void **con_cls)
 {
-	printf("url %s\n", url);
 	AVOutputFormat *oformat = av_guess_format(STREAM_MUX_NAME, nullptr, nullptr);
 	assert(oformat != nullptr);
 	HTTPD::Stream *stream = new HTTPD::Stream(oformat, width, height);
-	streams.push_back(stream);
+	{
+		unique_lock<mutex> lock(streams_mutex);
+		streams.insert(stream);
+	}
+	*con_cls = stream;
 
 	// Does not strictly have to be equal to MUX_BUFFER_SIZE.
 	MHD_Response *response = MHD_create_response_from_callback(
@@ -101,10 +109,32 @@ int HTTPD::answer_to_connection(MHD_Connection *connection,
 
 void HTTPD::free_stream(void *cls)
 {
+	// FIXME: When is this actually called, if ever?
+	// Also, shouldn't we remove it from streams?
 	HTTPD::Stream *stream = (HTTPD::Stream *)cls;
 	delete stream;
 }
 
+void HTTPD::request_completed_thunk(void *cls, struct MHD_Connection *connection, void **con_cls, enum MHD_RequestTerminationCode toe)
+{
+	HTTPD *httpd = (HTTPD *)cls;
+	return httpd->request_completed(connection, con_cls, toe);
+}
+
+void HTTPD::request_completed(struct MHD_Connection *connection, void **con_cls, enum MHD_RequestTerminationCode toe)
+{
+	if (con_cls == nullptr) {
+		// Request was never set up.
+		return;
+	}
+	HTTPD::Stream *stream = (HTTPD::Stream *)*con_cls;
+	{
+		unique_lock<mutex> lock(streams_mutex);
+		delete stream;
+		streams.erase(stream);
+	}
+}
+
 HTTPD::Mux::Mux(AVFormatContext *avctx, int width, int height)
 	: avctx(avctx)
 {
diff --git a/httpd.h b/httpd.h
index e8efc03..e739f70 100644
--- a/httpd.h
+++ b/httpd.h
@@ -16,8 +16,8 @@
 #include <deque>
 #include <memory>
 #include <mutex>
+#include <set>
 #include <string>
-#include <vector>
 
 struct MHD_Connection;
 
@@ -50,6 +50,10 @@ private:
 
 	static void free_stream(void *cls);
 
+	static void request_completed_thunk(void *cls, struct MHD_Connection *connection, void **con_cls, enum MHD_RequestTerminationCode toe);
+
+	void request_completed(struct MHD_Connection *connection, void **con_cls, enum MHD_RequestTerminationCode toe);
+
 	class Mux {
 	public:
 		Mux(AVFormatContext *avctx, int width, int height);  // Takes ownership of avctx.
@@ -74,15 +78,16 @@ private:
 		static int write_packet_thunk(void *opaque, uint8_t *buf, int buf_size);
 		int write_packet(uint8_t *buf, int buf_size);
 
-		std::unique_ptr<Mux> mux;
-
 		std::mutex buffer_mutex;
 		std::condition_variable has_buffered_data;
 		std::deque<std::string> buffered_data;  // Protected by <mutex>.
 		size_t used_of_buffered_data = 0;  // How many bytes of the first element of <buffered_data> that is already used. Protected by <mutex>.
+
+		std::unique_ptr<Mux> mux;  // Must come last to be destroyed before buffered_data, since the destructor can write bytes.
 	};
 
-	std::vector<Stream *> streams;  // Not owned.
+	std::mutex streams_mutex;
+	std::set<Stream *> streams;  // Not owned.
 
 	int width, height;
 	std::unique_ptr<Mux> file_mux;  // To local disk.