Fixed #7197: Segfault in linux CS after successful detach from database

src/jrd/ThreadCollect.h

@@ -0,0 +1,138 @@
+/*
+ *	PROGRAM:	JRD threading support
+ *	MODULE:		ThreadCollect.h
+ *	DESCRIPTION:	Threads' group completion handling
+ *
+ *  The contents of this file are subject to the Initial
+ *  Developer's Public License Version 1.0 (the "License");
+ *  you may not use this file except in compliance with the
+ *  License. You may obtain a copy of the License at
+ *  http://www.firebirdsql.org/en/initial-developer-s-public-license-version-1-0/
+ *
+ *  Software distributed under the License is distributed AS IS,
+ *  WITHOUT WARRANTY OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing rights
+ *  and limitations under the License.
+ *
+ *  The Original Code was created by Alexander Peshkov
+ *  for the Firebird Open Source RDBMS project.
+ *
+ *  Copyright (c) 2018, 2022 Alexander Peshkov <peshkoff@mail.ru>
+ *  and all contributors signed below.
+ *
+ *  All Rights Reserved.
+ *  Contributor(s): ______________________________________.
+ *
+ *
+ */
+
+#ifndef JRD_THREADCOLLECT_H
+#define JRD_THREADCOLLECT_H
+
+#include "../common/ThreadStart.h"
+#include "../common/classes/alloc.h"
+#include "../common/classes/array.h"
+#include "../common/classes/locks.h"
+
+
+namespace Jrd {
+
+	class ThreadCollect
+	{
+	public:
+		ThreadCollect(MemoryPool& p)
+			: threads(p)
+		{ }
+
+		void join()
+		{
+			if (!threads.hasData())
+				return;
+
+			waitFor(threads);
+		}
+
+		void ending(Thread::Handle& h)
+		{
+			// put thread into completion wait queue when it finished running
+			Firebird::MutexLockGuard g(threadsMutex, FB_FUNCTION);
+
+			for (unsigned n = 0; n < threads.getCount(); ++n)
+			{
+				if (threads[n].hndl == h)
+				{
+					threads[n].ending = true;
+					return;
+				}
+			}
+
+			Thrd t = {h, true};
+			threads.add(t);
+		}
+
+		void running(Thread::Handle& h)
+		{
+			// put thread into completion wait queue when it starts running
+			Firebird::MutexLockGuard g(threadsMutex, FB_FUNCTION);
+
+			Thrd t = {h, false};
+			threads.add(t);
+		}
+
+		void houseKeeping()
+		{
+			if (!threads.hasData())
+				return;
+
+			// join finished threads
+			AllThreads t;
+			{ // mutex scope
+				Firebird::MutexLockGuard g(threadsMutex, FB_FUNCTION);
+
+				for (unsigned n = 0; n < threads.getCount(); )
+				{
+					if (threads[n].ending)
+					{
+						t.add(threads[n]);
+						threads.remove(n);
+					}
+					else
+						++n;
+				}
+			}
+
+			waitFor(t);
+		}
+
+	private:
+		struct Thrd
+		{
+			Thread::Handle hndl;
+			bool ending;
+		};
+		typedef Firebird::HalfStaticArray<Thrd, 4> AllThreads;
+
+		void waitFor(AllThreads& thr)
+		{
+			Firebird::MutexLockGuard g(threadsMutex, FB_FUNCTION);
+			while (thr.hasData())
+			{
+				FB_SIZE_T n = thr.getCount() - 1;
+				Thrd& t = thr[n];
+				{
+					Firebird::MutexUnlockGuard u(threadsMutex, FB_FUNCTION);
+					Thread::waitForCompletion(t.hndl);
+					fb_assert(t.ending);
+				}
+				thr.remove(n);
+			}
+		}
+
+		AllThreads threads;
+		Firebird::Mutex threadsMutex;
+	};
+
+}	// namespace Jrd
+
+
+#endif	// JRD_THREADCOLLECT_H

src/jrd/jrd.cpp

@@ -110,6 +110,7 @@
 #include "../yvalve/why_proto.h"
 #include "../jrd/flags.h"
 #include "../jrd/Mapping.h"
+#include "../jrd/ThreadCollect.h"
 
 #include "../jrd/Database.h"
 
@@ -126,6 +127,7 @@
 #include "../common/classes/ClumpletWriter.h"
 #include "../common/classes/RefMutex.h"
 #include "../common/classes/ParsedList.h"
+#include "../common/classes/semaphore.h"
 #include "../common/utils_proto.h"
 #include "../jrd/DebugInterface.h"
 #include "../jrd/CryptoManager.h"
@@ -474,6 +476,16 @@ namespace
 {
 	using Jrd::Attachment;
 
+	// Required to sync attachment shutdown threads with provider shutdown
+	GlobalPtr<ThreadCollect> shutThreadCollect;
+
+	struct AttShutParams
+	{
+		Semaphore thdStartedSem;
+		Thread::Handle thrHandle;
+		AttachmentsRefHolder* attachments;
+	};
+
 	// Flag engineShutdown guarantees that no new attachment is created after setting it
 	// and helps avoid more than 1 shutdown threads running simultaneously.
 	bool engineShutdown = false;
@@ -4558,61 +4570,66 @@ void JProvider::shutdown(CheckStatusWrapper* status, unsigned int timeout, const
  **************************************/
 	try
 	{
-		MutexLockGuard guard(shutdownMutex, FB_FUNCTION);
-
-		if (engineShutdown)
-		{
-			return;
-		}
 		{ // scope
-			MutexLockGuard guard(newAttachmentMutex, FB_FUNCTION);
-			engineShutdown = true;
+			MutexLockGuard guard(shutdownMutex, FB_FUNCTION);
+
+			if (engineShutdown)
+			{
+				return;
+			}
+			{ // scope
+				MutexLockGuard guard(newAttachmentMutex, FB_FUNCTION);
+				engineShutdown = true;
+			}
+
+			ThreadContextHolder tdbb;
+
+			EDS::Manager::shutdown();
+
+			ULONG attach_count, database_count, svc_count;
+			JRD_enum_attachments(NULL, attach_count, database_count, svc_count);
+
+			if (attach_count > 0 || svc_count > 0)
+			{
+				gds__log("Shutting down the server with %d active connection(s) to %d database(s), "
+						 "%d active service(s)",
+					attach_count, database_count, svc_count);
+			}
+
+			if (reason == fb_shutrsn_exit_called)
+			{
+				// Starting threads may fail when task is going to close.
+				// This happens at least with some microsoft C runtimes.
+				// If people wish to have timeout, they should better call fb_shutdown() themselves.
+				// Therefore:
+				timeout = 0;
+			}
+
+			if (timeout)
+			{
+				Semaphore shutdown_semaphore;
+
+				Thread::Handle h;
+				Thread::start(shutdown_thread, &shutdown_semaphore, THREAD_medium, &h);
+
+				if (!shutdown_semaphore.tryEnter(0, timeout))
+					waitForShutdown(shutdown_semaphore);
+
+				Thread::waitForCompletion(h);
+			}
+			else
+			{
+				shutdown_thread(NULL);
+			}
+
+			// Do not put it into separate shutdown thread - during shutdown of TraceManager
+			// PluginManager wants to lock a mutex, which is sometimes already locked in current thread
+			TraceManager::shutdown();
+			Mapping::shutdownIpc();
 		}
 
-		ThreadContextHolder tdbb;
-
-		EDS::Manager::shutdown();
-
-		ULONG attach_count, database_count, svc_count;
-		JRD_enum_attachments(NULL, attach_count, database_count, svc_count);
-
-		if (attach_count > 0 || svc_count > 0)
-		{
-			gds__log("Shutting down the server with %d active connection(s) to %d database(s), "
-					 "%d active service(s)",
-				attach_count, database_count, svc_count);
-		}
-
-		if (reason == fb_shutrsn_exit_called)
-		{
-			// Starting threads may fail when task is going to close.
-			// This happens at least with some microsoft C runtimes.
-			// If people wish to have timeout, they should better call fb_shutdown() themselves.
-			// Therefore:
-			timeout = 0;
-		}
-
-		if (timeout)
-		{
-			Semaphore shutdown_semaphore;
-
-			Thread::Handle h;
-			Thread::start(shutdown_thread, &shutdown_semaphore, THREAD_medium, &h);
-
-			if (!shutdown_semaphore.tryEnter(0, timeout))
-				waitForShutdown(shutdown_semaphore);
-
-			Thread::waitForCompletion(h);
-		}
-		else
-		{
-			shutdown_thread(NULL);
-		}
-
-		// Do not put it into separate shutdown thread - during shutdown of TraceManager
-		// PluginManager wants to lock a mutex, which is sometimes already locked in current thread
-		TraceManager::shutdown();
-		Mapping::shutdownIpc();
+		// Wait for completion of all attacment shutdown threads
+		shutThreadCollect->join();
 	}
 	catch (const Exception& ex)
 	{
@@ -8672,22 +8689,26 @@ namespace
 		ThreadModuleRef thdRef(attachmentShutdownThread, &engineShutdown);
 #endif
 
+		AttShutParams* params = static_cast<AttShutParams*>(arg);
+		AttachmentsRefHolder* attachments = params->attachments;
+		Thread::Handle th = params->thrHandle;
+		fb_assert(th);
+
 		try
 		{
-			MutexLockGuard guard(shutdownMutex, FB_FUNCTION);
-			if (engineShutdown)
-			{
-				// Shutdown was done, all attachmnets are gone
-				return 0;
-			}
+			shutThreadCollect->running(th);
+			params->thdStartedSem.release();
 
-			shutdownAttachments(static_cast<AttachmentsRefHolder*>(arg), isc_att_shut_db_down);
+			MutexLockGuard guard(shutdownMutex, FB_FUNCTION);
+			if (!engineShutdown)
+				shutdownAttachments(attachments, isc_att_shut_db_down);
 		}
 		catch (const Exception& ex)
 		{
 			iscLogException("attachmentShutdownThread", ex);
 		}
 
+		shutThreadCollect->ending(th);
 		return 0;
 	}
 } // anonymous namespace
@@ -9474,13 +9495,18 @@ void JRD_shutdown_attachment(Attachment* attachment)
 		fb_assert(attachment->att_flags & ATT_shutdown);
 
 		MemoryPool& pool = *getDefaultMemoryPool();
-		AttachmentsRefHolder* queue = FB_NEW_POOL(pool) AttachmentsRefHolder(pool);
+		AutoPtr<AttachmentsRefHolder> queue(FB_NEW_POOL(pool) AttachmentsRefHolder(pool));
 
 		fb_assert(attachment->getStable());
 		attachment->getStable()->addRef();
 		queue->add(attachment->getStable());
 
-		Thread::start(attachmentShutdownThread, queue, THREAD_high);
+		AttShutParams params;
+		params.attachments = queue;
+		Thread::start(attachmentShutdownThread, &params, THREAD_high, &params.thrHandle);
+		queue.release();
+		shutThreadCollect->houseKeeping();
+		params.thdStartedSem.enter();
 	}
 	catch (const Exception&)
 	{} // no-op
@@ -9529,7 +9555,14 @@ void JRD_shutdown_attachments(Database* dbb)
 		}
 
 		if (queue.hasData())
-			Thread::start(attachmentShutdownThread, queue.release(), THREAD_high);
+		{
+			AttShutParams params;
+			params.attachments = queue;
+			Thread::start(attachmentShutdownThread, &params, THREAD_high, &params.thrHandle);
+			queue.release();
+			shutThreadCollect->houseKeeping();
+			params.thdStartedSem.enter();
+		}
 	}
 	catch (const Exception&)
 	{} // no-op

src/jrd/svc.cpp

@@ -74,6 +74,7 @@
 #include "../utilities/nbackup/nbkswi.h"
 #include "../jrd/trace/traceswi.h"
 #include "../jrd/val_proto.h"
+#include "../jrd/ThreadCollect.h"
 
 // Service threads
 #include "../burp/burp_proto.h"
@@ -120,6 +121,7 @@ int main_gstat(Firebird::UtilSvc* uSvc);
 
 
 using namespace Firebird;
+using namespace Jrd;
 
 const int SVC_user_dba			= 2;
 const int SVC_user_any			= 1;
@@ -138,64 +140,9 @@ namespace {
 	GlobalPtr<Mutex> globalServicesMutex;
 
 	// All that we need to shutdown service threads when shutdown in progress
-	typedef Array<Jrd::Service*> AllServices;
+	typedef Array<Service*> AllServices;
 	GlobalPtr<AllServices> allServices;	// protected by globalServicesMutex
 	volatile bool svcShutdown = false;
-
-	class ThreadCollect
-	{
-	public:
-		ThreadCollect(MemoryPool& p)
-			: threads(p)
-		{ }
-
-		void join()
-		{
-			// join threads to be sure they are gone when shutdown is complete
-			// no need locking something cause this is expected to run when services are closing
-			waitFor(threads);
-		}
-
-		void add(const Thread::Handle& h)
-		{
-			// put thread into completion wait queue when it finished running
-			MutexLockGuard g(threadsMutex, FB_FUNCTION);
-			fb_assert(h);
-			threads.add(h);
-		}
-
-		void houseKeeping()
-		{
-			if (!threads.hasData())
-				return;
-
-			// join finished threads
-			AllThreads t;
-			{ // mutex scope
-				MutexLockGuard g(threadsMutex, FB_FUNCTION);
-				t.assign(threads);
-				threads.clear();
-			}
-
-			waitFor(t);
-		}
-
-	private:
-		typedef Array<Thread::Handle> AllThreads;
-
-		static void waitFor(AllThreads& thr)
-		{
-			while (thr.hasData())
-			{
-				Thread::Handle h(thr.pop());
-				Thread::waitForCompletion(h);
-			}
-		}
-
-		AllThreads threads;
-		Mutex threadsMutex;
-	};
-
 	GlobalPtr<ThreadCollect> threadCollect;
 
 	void spbVersionError()
@@ -207,8 +154,6 @@ namespace {
 } // anonymous namespace
 
 
-using namespace Jrd;
-
 namespace {
 const serv_entry services[] =
 {
@@ -1975,7 +1920,7 @@ THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg)
 		svc->unblockQueryGet();
 		svc->finish(SVC_finished);
 
-		threadCollect->add(thrHandle);
+		threadCollect->ending(thrHandle);
 	}
 	catch (const Exception& ex)
 	{