Make sure Spawner RAII destructors don't re-throw thread_interrupted,…

… and document this issue.

CONTRIBUTING.md

@@ -280,3 +280,7 @@ Be careful with event loop callbacks, they are more tricky than one would expect
         }
 
  * Event loop callbacks should catch expected exceptions. Letting an exception pass will crash the program. When system call failure simulation is turned on, the code can throw arbitrary SystemExceptions, so beware of those.
+
+### Thread interruption and RAII destructors
+
+When using thread interruption, make sure that RAII destructors are non-interruptable. If your code is interrupted and then a `thread_interrupted` is thrown, make sure that RAII destructors don't check for the interruption flag and then throw `thread_interrupted` again. This not only fails to clean things up properly, but also confuses the exception system, resulting in strange errors such as "terminate called without an active exception".

ext/common/ApplicationPool2/Implementation.cpp

@@ -229,9 +229,10 @@ Group::onSessionInitiateFailure(const ProcessPtr &process, Session *session) {
 	}
 
 	UPDATE_TRACE_POINT();
-	if (pool->detachProcessUnlocked(process, actions)) {
-		P_DEBUG("Could not initiate a session with process " <<
-			process->inspect() << ", detached from pool");
+	P_DEBUG("Could not initiate a session with process " <<
+		process->inspect() << ", detaching from pool if possible");
+	if (!pool->detachProcessUnlocked(process, actions)) {
+		P_DEBUG("Process was already detached");
 	}
 	pool->fullVerifyInvariants();
 	lock.unlock();

ext/common/ApplicationPool2/Spawner.h

@@ -1282,6 +1282,8 @@ class SmartSpawner: public Spawner, public enable_shared_from_this<SmartSpawner>
 
 	void startPreloader() {
 		TRACE_POINT();
+		this_thread::disable_interruption di;
+		this_thread::disable_syscall_interruption dsi;
 		assert(!preloaderStarted());
 		checkChrootDirectories(options);
 
@@ -1344,7 +1346,11 @@ class SmartSpawner: public Spawner, public enable_shared_from_this<SmartSpawner>
 			details.timeout = options.startTimeout * 1000;
 			details.forwardStderr = forwardStderr;
 
-			socketAddress = negotiatePreloaderStartup(details);
+			{
+				this_thread::restore_interruption ri(di);
+				this_thread::restore_syscall_interruption rsi(dsi);
+				socketAddress = negotiatePreloaderStartup(details);
+			}
 			this->adminSocket = adminSocket.second;
 			{
 				lock_guard<boost::mutex> l(simpleFieldSyncher);
@@ -1997,6 +2003,8 @@ class DirectSpawner: public Spawner {
 
 	virtual ProcessPtr spawn(const Options &options) {
 		TRACE_POINT();
+		this_thread::disable_interruption di;
+		this_thread::disable_syscall_interruption dsi;
 		P_DEBUG("Spawning new process: appRoot=" << options.appRoot);
 		possiblyRaiseInternalError(options);
 
@@ -2062,7 +2070,12 @@ class DirectSpawner: public Spawner {
 			details.forwardStderr = forwardStderr;
 			details.debugDir = debugDir;
 
-			ProcessPtr process = negotiateSpawn(details);
+			ProcessPtr process;
+			{
+				this_thread::restore_interruption ri(di);
+				this_thread::restore_syscall_interruption rsi(dsi);
+				process = negotiateSpawn(details);
+			}
 			detachProcess(process->pid);
 			guard.clear();
 			P_DEBUG("Process spawning done: appRoot=" << options.appRoot <<