From 9bbff324ec27c0a110ab173c53111246cdb7fe80 Mon Sep 17 00:00:00 2001
From: emeric <emeric@nas>
Date: Wed, 20 Aug 2014 01:00:00 +0200
Subject: [PATCH] Try to avoid avconv zombies

---
 TODO                           |  1 +
 service/ServiceManager.cpp     |  5 +++
 transcode/AvConvTranscoder.cpp | 71 ++++++++++++++++++++--------------
 transcode/AvConvTranscoder.hpp |  3 ++
 4 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/TODO b/TODO
index 8eea6970..42c48858 100644
--- a/TODO
+++ b/TODO
@@ -33,6 +33,7 @@
 
 [UI]
 	[Settings]
+		- increase the menu's width
 		- logout users that are being changed (loss of admin admin rights), or make sure they are still admin when they make changes
 		- "signal not exposed" problem if a user logout and login again. bad resource destruction?
 		[admin/DB]
diff --git a/service/ServiceManager.cpp b/service/ServiceManager.cpp
index 7ec84c10..59a50e6e 100644
--- a/service/ServiceManager.cpp
+++ b/service/ServiceManager.cpp
@@ -23,6 +23,11 @@ ServiceManager::ServiceManager()
 #endif // defined(SIGQUIT)
 
 	_signalSet.add(SIGHUP);
+
+	// Excplicitely ignore SIGCHLD to avoid zombies
+	// when avconv child processes are being killed
+	if (::signal(SIGCHLD, SIG_IGN) == SIG_ERR)
+		throw std::runtime_error("ServiceManager::ServiceManager, signal failed!");
 }
 
 ServiceManager::~ServiceManager()
diff --git a/transcode/AvConvTranscoder.cpp b/transcode/AvConvTranscoder.cpp
index 678a8627..be20af38 100644
--- a/transcode/AvConvTranscoder.cpp
+++ b/transcode/AvConvTranscoder.cpp
@@ -11,6 +11,8 @@
 namespace Transcode
 {
 
+boost::mutex	AvConvTranscoder::_mutex;
+
 boost::filesystem::path	AvConvTranscoder::_avConvPath = "";
 
 void
@@ -109,15 +111,21 @@ AvConvTranscoder::AvConvTranscoder(const Parameters& parameters)
 
 	std::cout << "executing... '" << oss.str() << "'" << std::endl;
 
-	std::vector<int> ranges = { 3, 1024 };	// fd range to be closed
-	_child = std::make_shared<boost::process::child>( boost::process::execute(
-				boost::process::initializers::run_exe(_avConvPath),
-				boost::process::initializers::set_cmd_line(oss.str()),
-				boost::process::initializers::bind_stdout(sink),
-				boost::process::initializers::close_fd(STDIN_FILENO),
-				boost::process::initializers::close_fds(ranges)
-				)
-			);
+	// make sure only one thread is executing this part of code
+	// See boost process FAQ
+	{
+		boost::lock_guard<boost::mutex>	lock(_mutex);
+		std::vector<int> ranges = { 3, 1024 };	// fd range to be closed
+
+		_child = std::make_shared<boost::process::child>( boost::process::execute(
+					boost::process::initializers::run_exe(_avConvPath),
+					boost::process::initializers::set_cmd_line(oss.str()),
+					boost::process::initializers::bind_stdout(sink),
+					boost::process::initializers::close_fd(STDIN_FILENO),
+					boost::process::initializers::close_fds(ranges)
+					)
+				);
+	}
 
 }
 
@@ -154,34 +162,37 @@ AvConvTranscoder::~AvConvTranscoder()
 void
 AvConvTranscoder::waitChild()
 {
-	try {
-		if (_child) {
-			std::cout << "waiting for child!" << std::endl;
-			boost::process::wait_for_exit(*_child);
-			std::cout << "waiting for child! DONE" << std::endl;
-			_child.reset();
-		}
-	}
-	catch( std::exception& e)
+	if (_child)
 	{
-		std::cerr << "Exception caugh in waitChild: " << e.what() << std::endl;
+		boost::system::error_code ec;
+
+		std::cout << "waiting for child!" << std::endl;
+		boost::process::wait_for_exit(*_child, ec);
+		std::cout << "waiting for child! DONE." << std::endl;
+
+		if (ec)
+			std::cerr << "AvConvTranscoder::waitChild: error: " << ec.message() << std::endl;
+
+		_child.reset();
 	}
 }
 
-	void
+void
 AvConvTranscoder::killChild()
 {
-	try {
-		if (_child) {
-			std::cout << "Killing child!" << std::endl;
-			boost::process::terminate(*_child);
-			std::cout << "Killing child DONE" << std::endl;
-			_child.reset();
-		}
-	}
-	catch( std::exception& e)
+	if (_child)
 	{
-		std::cerr << "Exception caugh in killChild: " << e.what() << std::endl;
+		boost::system::error_code ec;
+
+		std::cout << "Killing child! pid = " << _child->pid << std::endl;
+		boost::process::terminate(*_child, ec);
+		std::cout << "Killing child DONE" << std::endl;
+
+		// If an error occured, force kill the child
+		if (ec)
+			std::cerr << "AvConvTranscoder::killChild: error: " << ec.message() << std::endl;
+
+		_child.reset();
 	}
 }
 
diff --git a/transcode/AvConvTranscoder.hpp b/transcode/AvConvTranscoder.hpp
index 8e8ee018..5ce77921 100644
--- a/transcode/AvConvTranscoder.hpp
+++ b/transcode/AvConvTranscoder.hpp
@@ -8,6 +8,7 @@
 #include <boost/iostreams/stream.hpp>
 #include <boost/process.hpp>
 #include <boost/filesystem.hpp>
+#include <boost/thread.hpp>
 
 #include "Parameters.hpp"
 
@@ -43,6 +44,8 @@ class AvConvTranscoder
 
 		const Parameters	_parameters;
 
+		static boost::mutex	_mutex;
+
 		boost::process::pipe _outputPipe;
 		boost::iostreams::file_descriptor_source _source;
 		boost::iostreams::stream_buffer<boost::iostreams::file_descriptor_source> _is;