Fixed CORE-2728: Access Violation when trying to add an user that alr…

…eady exists

src/common/fb_exception.cpp

@@ -29,7 +29,7 @@ class StringsBuffer
 		FB_THREAD_ID thread;
 
 	public:
-		ThreadBuffer() : buffer_ptr(buffer), thread(getThreadId()) { }
+		ThreadBuffer(FB_THREAD_ID thr) : buffer_ptr(buffer), thread(thr) { }
 
 		const char* alloc(const char* string, size_t& length)
 		{
@@ -54,10 +54,8 @@ class StringsBuffer
 			return new_string;
 		}
 
-		bool thisThread()
+		bool thisThread(FB_THREAD_ID currTID)
 		{
-			const FB_THREAD_ID currTID = getThreadId();
-
 #ifdef WIN_NT
 			if (thread != currTID)
 			{
@@ -98,13 +96,13 @@ class StringsBuffer
 	}
 
 private:
-	size_t position()
+	size_t position(FB_THREAD_ID thr)
 	{
 		// mutex should be locked when this function is called
 
 		for (size_t i = 0; i < processBuffer.getCount(); ++i)
 		{
-			if (processBuffer[i]->thisThread())
+			if (processBuffer[i]->thisThread(thr))
 			{
 				return i;
 			}
@@ -113,17 +111,17 @@ class StringsBuffer
 		return processBuffer.getCount();
 	}
 
-	ThreadBuffer* getThreadBuffer()
+	ThreadBuffer* getThreadBuffer(FB_THREAD_ID thr)
 	{
 		Firebird::MutexLockGuard guard(mutex);
 
-		size_t p = position();
+		size_t p = position(thr);
 		if (p < processBuffer.getCount())
 		{
 			return processBuffer[p];
 		}
 
-		ThreadBuffer* b = new ThreadBuffer;
+		ThreadBuffer* b = new ThreadBuffer(thr);
 		processBuffer.add(b);
 		return b;
 	}
@@ -132,7 +130,7 @@ class StringsBuffer
 	{
 		Firebird::MutexLockGuard guard(mutex);
 
-		size_t p = position();
+		size_t p = position(getThreadId());
 		if (p >= processBuffer.getCount())
 		{
 			return;
@@ -148,10 +146,10 @@ class StringsBuffer
 	}
 
 public:
-	const char* alloc(const char* s, size_t& len)
+	const char* alloc(const char* s, size_t& len, FB_THREAD_ID thr = getThreadId())
 	{
 		ThreadCleanup::add(cleanupAllStrings, this);
-		return getThreadBuffer()->alloc(s, len);
+		return getThreadBuffer(thr)->alloc(s, len);
 	}
 };
 
@@ -161,9 +159,11 @@ Firebird::GlobalPtr<StringsBuffer> allStrings;
 
 namespace Firebird {
 
+// Before using thr parameter, make sure that thread is not going to work with
+// this functions itself.
 // CVC: Do not let "perm" be incremented before "trans", because it may lead to serious memory errors,
 // since several places in our code blindly pass the same vector twice.
-void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans) throw()
+void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans, FB_THREAD_ID thr) throw()
 {
 	try
 	{
@@ -179,7 +179,7 @@ void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans) throw()
 				{
 					size_t len = *perm++ = *trans++;
 					const char* temp = reinterpret_cast<char*>(*trans++);
-					*perm++ = (ISC_STATUS)(IPTR) (allStrings->alloc(temp, len));
+					*perm++ = (ISC_STATUS)(IPTR) (allStrings->alloc(temp, len, thr));
 					//perm[-2] = len; redundant
 				}
 				break;
@@ -189,7 +189,7 @@ void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans) throw()
 				{
 					const char* temp = reinterpret_cast<char*>(*trans++);
 					size_t len = strlen(temp);
-					*perm++ = (ISC_STATUS)(IPTR) (allStrings->alloc(temp, len));
+					*perm++ = (ISC_STATUS)(IPTR) (allStrings->alloc(temp, len, thr));
 				}
 				break;
 			default:
@@ -216,9 +216,9 @@ void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans) throw()
 	}
 }
 
-void makePermanentVector(ISC_STATUS* v) throw()
+void makePermanentVector(ISC_STATUS* v, FB_THREAD_ID thr) throw()
 {
-	makePermanentVector(v, v);
+	makePermanentVector(v, v, thr);
 }
 
 // ********************************* Exception *******************************

src/common/thd.cpp

@@ -61,7 +61,7 @@
 #include <pthread.h>
 #endif
 
-FB_THREAD_ID getThreadId()
+FB_THREAD_ID getThreadId() throw()
 {
 /**************************************
  *

src/common/thd.h

@@ -35,7 +35,7 @@ void	THD_sleep(ULONG);
 void	THD_yield();
 
 // thread ID
-FB_THREAD_ID getThreadId();
+FB_THREAD_ID getThreadId() throw();
 
 class ThreadCleanup
 {

src/include/fb_exception.h

@@ -35,6 +35,7 @@
 #include <new>
 #include "fb_types.h"
 #include "../common/StatusArg.h"
+#include "../common/thd.h"
 
 namespace Firebird {
 
@@ -144,8 +145,8 @@ class fatal_exception : public status_exception
 ISC_STATUS stuff_exception(ISC_STATUS *status_vector, const Firebird::Exception& ex) throw();
 
 // Put status vector strings into strings buffer
-void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans) throw();
-void makePermanentVector(ISC_STATUS* v) throw();
+void makePermanentVector(ISC_STATUS* perm, const ISC_STATUS* trans, FB_THREAD_ID thr = getThreadId()) throw();
+void makePermanentVector(ISC_STATUS* v, FB_THREAD_ID thr = getThreadId()) throw();
 
 }	// namespace Firebird
 

src/jrd/svc.cpp

@@ -125,6 +125,24 @@ const char* const SPB_SEC_USERNAME = "isc_spb_sec_username";
 
 namespace {
 
+	// Thread ID holder (may be generic, but needed only here now)
+	class ThreadIdHolder
+	{
+	public:
+		ThreadIdHolder(Jrd::Service::StatusStringsHelper& p) : strHelper(&p)
+		{
+			MutexLockGuard guard(strHelper->mtx);
+			strHelper->workerThread = getThreadId();
+		}
+		~ThreadIdHolder()
+		{
+			MutexLockGuard guard(strHelper->mtx);
+			strHelper->workerThread = 0;
+		}
+	private:
+		Jrd::Service::StatusStringsHelper* strHelper;
+	};
+
 	// Option block for service parameter block
 	struct Options
 	{
@@ -418,6 +436,21 @@ void Service::putBytes(const UCHAR* bytes, size_t len)
 	enqueue(bytes, len);
 }
 
+void Service::makePermanentStatusVector() throw()
+{
+	// This mutex avoids modification of workerThread
+	MutexLockGuard guard(svc_thread_strings.mtx);
+
+	if (svc_thread_strings.workerThread)
+	{
+		makePermanentVector(svc_status, svc_thread_strings.workerThread);
+	}
+	else
+	{
+		makePermanentVector(svc_status);
+	}
+}
+
 void Service::setServiceStatus(const ISC_STATUS* status_vector)
 {
 	if (checkForShutdown())
@@ -431,7 +464,7 @@ void Service::setServiceStatus(const ISC_STATUS* status_vector)
 		Arg::StatusVector passed(status_vector);
 		svc.append(passed);
 		svc.copyTo(svc_status);
-		makePermanentVector(svc_status);
+		makePermanentStatusVector();
 	}
 }
 
@@ -531,7 +564,7 @@ void Service::setServiceStatus(const USHORT facility, const USHORT errcode,
 		}
 	}
 
-	makePermanentVector(svc_status);
+	makePermanentStatusVector();
 }
 
 void Service::hidePasswd(ArgvType&, int)
@@ -702,6 +735,7 @@ Service::Service(const TEXT* service_name, USHORT spb_length, const UCHAR* spb_d
 {
 	svc_trace_manager = NULL;
 	memset(svc_status, 0, sizeof svc_status);
+	ThreadIdHolder holdId(svc_thread_strings);
 
 	{	// scope
 		// Account service block in global array
@@ -916,6 +950,8 @@ static THREAD_ENTRY_DECLARE svcShutdownThread(THREAD_ENTRY_PARAM)
 
 void Service::detach()
 {
+	ThreadIdHolder holdId(svc_thread_strings);
+
 	// save it cause after call to finish() we can't access class members any more
 	const bool localDoShutdown = svc_do_shutdown;
 
@@ -1027,6 +1063,8 @@ ISC_STATUS Service::query2(thread_db* tdbb,
 	UCHAR buffer[MAXPATHLEN];
 	USHORT l, length, version, get_flags;
 
+	ThreadIdHolder holdId(svc_thread_strings);
+
 	// Setup the status vector
 	Arg::StatusVector status;
 
@@ -1891,6 +1929,8 @@ void Service::query(USHORT			send_item_length,
 
 void Service::start(USHORT spb_length, const UCHAR* spb_data)
 {
+	ThreadIdHolder holdId(svc_thread_strings);
+
 	try
 	{
 	ClumpletReader spb(ClumpletReader::SpbStart, spb_data, spb_length);

src/jrd/svc.h

@@ -233,6 +233,8 @@ class Service : public Firebird::UtilSvc, public TypedHandle<type_svc>
 	static void need_admin_privs(Firebird::Arg::StatusVector& status, const char* message);
 	// Does info buffer have enough space for SLONG?
 	static bool ck_space_for_numeric(UCHAR*& info, const UCHAR* const end);
+	// Make status vector permamnent, if one present in worker thread's space
+	void makePermanentStatusVector() throw();
 
 private:
 	ISC_STATUS_ARRAY svc_status;		// status vector for running service
@@ -268,6 +270,15 @@ class Service : public Firebird::UtilSvc, public TypedHandle<type_svc>
 	SLONG				svc_remote_pid;
 
 	TraceManager*		svc_trace_manager;
+
+public:
+	struct StatusStringsHelper
+	{
+		FB_THREAD_ID workerThread;
+		Firebird::Mutex mtx;
+	};
+private:
+	StatusStringsHelper	svc_thread_strings;
 };
 
 } //namespace Jrd