Fixed CORE-4303: Possible races while Service destruction, related code cleanup

src/alice/alice.cpp

@@ -93,9 +93,8 @@ static void alice_output(bool error, const SCHAR*, ...) ATTRIBUTE_FORMAT(2,3);
 //	Entry point for GFIX in case of service manager.
 //
 
-THREAD_ENTRY_DECLARE ALICE_main(THREAD_ENTRY_PARAM arg)
+int ALICE_main(Firebird::UtilSvc* uSvc)
 {
-	Firebird::UtilSvc* uSvc = (Firebird::UtilSvc*) arg;
 	int exit_code = FINI_OK;
 
 	try {
@@ -110,8 +109,7 @@ THREAD_ENTRY_DECLARE ALICE_main(THREAD_ENTRY_PARAM arg)
 		exit_code = FB_FAILURE;
 	}
 
-	uSvc->finish();
+	return exit_code;
-	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
 }
 
 //____________________________________________________________

src/alice/alice_proto.h

@@ -28,7 +28,7 @@
 #include "../common/classes/MsgPrint.h"
 #include "../common/UtilSvc.h"
 
-THREAD_ENTRY_DECLARE ALICE_main(THREAD_ENTRY_PARAM);
+int		ALICE_main(Firebird::UtilSvc*);
 int		alice(Firebird::UtilSvc*);
 
 class AliceGlobals;

src/burp/burp.cpp

@@ -117,7 +117,7 @@ const ULONG MBYTE	= KBYTE * KBYTE;
 const ULONG GBYTE	= MBYTE * KBYTE;
 
 
-THREAD_ENTRY_DECLARE BURP_main(THREAD_ENTRY_PARAM arg)
+int BURP_main(Firebird::UtilSvc* uSvc)
 {
 /**************************************
  *
@@ -129,7 +129,6 @@ THREAD_ENTRY_DECLARE BURP_main(THREAD_ENTRY_PARAM arg)
  *	Entrypoint for GBAK via services manager.
  *
  **************************************/
-	Firebird::UtilSvc* uSvc = (Firebird::UtilSvc*) arg;
 	int exit_code = FINI_OK;
 
 	try {
@@ -144,8 +143,7 @@ THREAD_ENTRY_DECLARE BURP_main(THREAD_ENTRY_PARAM arg)
 		exit_code = FB_FAILURE;
 	}
 
-	uSvc->finish();
+	return exit_code;
-	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
 }
 
 

src/burp/burp_proto.h

@@ -28,7 +28,7 @@
 #include "../common/classes/MsgPrint.h"
 #include "../common/UtilSvc.h"
 
-THREAD_ENTRY_DECLARE BURP_main(THREAD_ENTRY_PARAM);
+int		BURP_main(Firebird::UtilSvc*);
 int		gbak(Firebird::UtilSvc*);
 
 void	BURP_abort();

src/common/UtilSvc.cpp

@@ -61,8 +61,7 @@ namespace {
 class StandaloneUtilityInterface : public UtilSvc
 {
 public:
-	StandaloneUtilityInterface(int ac, char** av) :
+	StandaloneUtilityInterface(int ac, char** av)
-	  m_finished(false)
 	{
 		while (ac--)
 		{
@@ -131,7 +130,6 @@ public:
 	}
 
 	// do nothing for non-service
-	virtual void finish() { m_finished = true; }
 	virtual void started() { }
 	virtual void putLine(char, const char*) { }
 	virtual void putSLong(char, SLONG) { }
@@ -142,11 +140,8 @@ public:
 	virtual void setServiceStatus(const USHORT, const USHORT, const MsgFormat::SafeArg&) { }
     virtual const ISC_STATUS* getStatus() { return 0; }
 	virtual void fillDpb(ClumpletWriter&) { }
-	virtual bool finished() { return m_finished; };
+	virtual bool finished() { return false; };
 	virtual void initStatus() { }
-
-private:
-	bool m_finished;
 };
 
 

src/common/UtilSvc.h

@@ -54,7 +54,6 @@ public:
 
 	virtual bool isService() = 0;
 	virtual void started() = 0;
-	virtual void finish() = 0;
 	virtual void outputVerbose(const char* text) = 0;
 	virtual void outputError(const char* text) = 0;
 	virtual void outputData(const void* text, size_t size) = 0;
@@ -110,7 +109,6 @@ protected:
 	bool usvcDataMode;
 };
 
-
 } // namespace Firebird
 
 #endif // FB_UTILFACE

src/jrd/EngineInterface.h

@@ -442,10 +442,10 @@ public:
 
 public:
 	explicit JService(Service* handle);
-
+	Firebird::Mutex mutex;
-private:
 	Service* svc;
 
+private:
 	void freeEngineData(Firebird::IStatus* status);
 };
 

src/jrd/jrd.cpp

@@ -3692,24 +3692,26 @@ JService* JProvider::attachServiceManager(IStatus* user_status, const char* serv
  *	Connect to a Firebird service.
  *
  **************************************/
-	JService* svc = NULL;
+	JService* jSvc = NULL;
 
 	try
 	{
 		ThreadContextHolder tdbb(user_status);
 
-		svc = new JService(new Service(service_name, spbLength, spb, cryptCallback));
+		Service* svc = new Service(service_name, spbLength, spb, cryptCallback);
-		svc->addRef();
+		jSvc = new JService(svc);
+		svc->jSvc = jSvc;
+		jSvc->addRef();
 	}
 	catch (const Exception& ex)
 	{
 		ex.stuffException(user_status);
-		return svc;
+		return jSvc;
 	}
 
 	successful_completion(user_status);
 
-	return svc;
+	return jSvc;
 }
 
 

src/jrd/svc.cpp

@@ -63,6 +63,7 @@
 #include "../jrd/trace/TraceObjects.h"
 #include "../jrd/EngineInterface.h"
 #include "../jrd/Mapping.h"
+#include "../common/classes/RefMutex.h"
 
 #include "../common/classes/DbImplementation.h"
 
@@ -188,8 +189,8 @@ namespace {
 
 using namespace Jrd;
 
-Service::ExistenceGuard::ExistenceGuard(Service* s, const char* from)
+Service::SafeMutexLock::SafeMutexLock(Service* svc, const char* f)
-	: svc(s), locked(false)
+	: from(f)
 {
 	MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
 
@@ -205,29 +206,69 @@ Service::ExistenceGuard::ExistenceGuard(Service* s, const char* from)
 		Arg::Gds(isc_bad_svc_handle).raise();
 	}
 
-	// Appears we have correct handle, lock it to make sure service exists
+	// Appears we have correct service object, may use it later to lock mutex
-	// for our lifetime
+	jSvc = svc->jSvc;
-	svc->svc_existence_lock.enter(from);
+}
-	fb_assert(!svc->svc_current_guard);
+
-	svc->svc_current_guard = this;
+bool Service::SafeMutexLock::lock()
-	locked = true;
+{
+	jSvc->mutex.enter(from);
+	return jSvc->svc;
+}
+
+Service::ExistenceGuard::ExistenceGuard(Service* svc, const char* from)
+	: SafeMutexLock(svc, from)
+{
+	if (!lock())
+	{
+		// could not lock service
+		Arg::Gds(isc_bad_svc_handle).raise();
+	}
 }
 
 Service::ExistenceGuard::~ExistenceGuard()
 {
-	release();
+	try
-}
-
-void Service::ExistenceGuard::release()
-{
-	if (locked)
 	{
-		locked = false;
+		jSvc->mutex.leave();
-		svc->svc_current_guard = NULL;
+	}
-		svc->svc_existence_lock.leave();
+	catch(const Exception&)
+	{
+		DtorException::devHalt();
 	}
 }
 
+Service::UnlockGuard::UnlockGuard(Service* svc, const char* from)
+	: SafeMutexLock(svc, from), locked(true), doLock(true)
+{
+	jSvc->mutex.leave();
+	locked = false;
+}
+
+bool Service::UnlockGuard::enter()
+{
+	doLock = false;
+	if (!locked)
+	{
+		if (!lock())
+		{
+			return false;
+		}
+		locked = true;
+	}
+	return true;
+}
+
+Service::UnlockGuard::~UnlockGuard()
+{
+	if (doLock && (!locked) && (!lock()))
+	{
+		// could not lock service
+		DtorException::devHalt();
+	}
+}
+
+
 void Service::getOptions(ClumpletReader& spb)
 {
 	svc_spb_version = spb.getBufferTag();
@@ -425,20 +466,16 @@ bool Service::isService()
 
 void Service::started()
 {
+	// ExistenceGuard guard(this, FB_FUNCTION);
+	// Not needed here - lock is taken by thread waiting for us
+
 	if (!(svc_flags & SVC_evnt_fired))
 	{
-		MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
 		svc_flags |= SVC_evnt_fired;
 		svcStart.release();
 	}
 }
 
-void Service::finish()
-{
-	svc_sem_full.release();
-	finish(SVC_finished);
-}
-
 void Service::putLine(char tag, const char* val)
 {
 	const ULONG len = strlen(val) & 0xFFFF;
@@ -709,14 +746,14 @@ Service::Service(const TEXT* service_name, USHORT spb_length, const UCHAR* spb_d
 	: svc_parsed_sw(getPool()),
 	svc_stdout_head(0), svc_stdout_tail(0), svc_service(NULL), svc_service_run(NULL),
 	svc_resp_alloc(getPool()), svc_resp_buf(0), svc_resp_ptr(0), svc_resp_buf_len(0),
-	svc_resp_len(0), svc_flags(0), svc_user_flag(0), svc_spb_version(0), svc_do_shutdown(false),
+	svc_resp_len(0), svc_flags(SVC_finished), svc_user_flag(0), svc_spb_version(0),
+	svc_do_shutdown(false), svc_shutdown_in_progress(false), svc_timeout(false),
 	svc_username(getPool()), svc_auth_block(getPool()),
 	svc_expected_db(getPool()), svc_trusted_role(false), svc_utf8(false),
 	svc_switches(getPool()), svc_perm_sw(getPool()), svc_address_path(getPool()),
 	svc_command_line(getPool()),
 	svc_network_protocol(getPool()), svc_remote_address(getPool()), svc_remote_process(getPool()),
 	svc_remote_pid(0), svc_trace_manager(NULL), svc_crypt_callback(crypt_callback),
-	svc_current_guard(NULL),
 	svc_stdin_size_requested(0), svc_stdin_buffer(NULL), svc_stdin_size_preload(0),
 	svc_stdin_preload_requested(0), svc_stdin_user_size(0)
 {
@@ -822,7 +859,7 @@ Service::Service(const TEXT* service_name, USHORT spb_length, const UCHAR* spb_d
 			switches += ' ';
 		switches += svc_command_line;
 
-		svc_flags = switches.hasData() ? SVC_cmd_line : 0;
+		svc_flags |= switches.hasData() ? SVC_cmd_line : 0;
 		svc_perm_sw = switches;
 		svc_user_flag = user_flag;
 		svc_service = serv;
@@ -833,7 +870,7 @@ Service::Service(const TEXT* service_name, USHORT spb_length, const UCHAR* spb_d
 		// Only do this if we are working with a version 1 service
 		if (serv->serv_thd && svc_spb_version == isc_spb_version1)
 		{
-			start(serv->serv_thd);
+			start(serv);
 		}
 	}	// try
 	catch (const Firebird::Exception& ex)
@@ -917,11 +954,6 @@ Service::~Service()
 
 	delete svc_trace_manager;
 	svc_trace_manager = NULL;
-
-	if (svc_current_guard)
-	{
-		svc_current_guard->release();
-	}
 }
 
 
@@ -982,15 +1014,13 @@ bool Service::checkForShutdown()
 {
 	if (svcShutdown)
 	{
-		MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
+		if (svc_shutdown_in_progress)
-
-		if (svc_flags & SVC_shutdown)
 		{
 			// Here we avoid multiple exceptions thrown
 			return true;
 		}
 
-		svc_flags |= SVC_shutdown;
+		svc_shutdown_in_progress = true;
 		status_exception::raise(Arg::Gds(isc_att_shutdown));
 	}
 
@@ -1010,7 +1040,7 @@ void Service::shutdownServices()
 	// signal once for every still running service
 	for (pos = 0; pos < all.getCount(); pos++)
 	{
-		if (all[pos]->svc_flags & SVC_thd_running)
+		if (!(all[pos]->svc_flags & SVC_finished))
 			all[pos]->svc_detach_sem.release();
 		if (all[pos]->svc_stdin_size_requested)
 			all[pos]->svc_stdin_semaphore.release();
@@ -1018,7 +1048,7 @@ void Service::shutdownServices()
 
 	for (pos = 0; pos < all.getCount(); )
 	{
-		if (all[pos]->svc_flags & SVC_thd_running)
+		if (!(all[pos]->svc_flags & SVC_finished))
 		{
 			globalServicesMutex->leave();
 			THD_sleep(1);
@@ -1290,18 +1320,15 @@ ISC_STATUS Service::query2(thread_db* /*tdbb*/,
 			break;
 
 		case isc_info_svc_running:
-			// Returns the status of the flag SVC_thd_running
+			// Returns the (inversed) status of the flag SVC_finished
 			if (!ck_space_for_numeric(info, end))
 				return 0;
 			*info++ = item;
-			{	// guardian scope
-				MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
 
-				if (svc_flags & SVC_thd_running)
+			if (svc_flags & SVC_finished)
-					ADD_SPB_NUMERIC(info, TRUE)
+				ADD_SPB_NUMERIC(info, FALSE)
-				else
+			else
-					ADD_SPB_NUMERIC(info, FALSE)
+				ADD_SPB_NUMERIC(info, TRUE)
-			}
 
 			break;
 
@@ -1461,7 +1488,7 @@ ISC_STATUS Service::query2(thread_db* /*tdbb*/,
 				return 0;
 			}
 
-			if (svc_flags & SVC_timeout)
+			if (svc_timeout)
 			{
 				*info++ = isc_info_svc_timeout;
 			}
@@ -1549,11 +1576,6 @@ ISC_STATUS Service::query2(thread_db* /*tdbb*/,
 		throw;
 	}
 
-	if (!(svc_flags & SVC_thd_running))
-	{
-		finish(SVC_finished);
-	}
-
 	return svc_status[1];
 }
 
@@ -1892,7 +1914,7 @@ void Service::query(USHORT			send_item_length,
 
 			info = INF_put_item(item, length, info + 3, info, end);
 
-			if (svc_flags & SVC_timeout)
+			if (svc_timeout)
 				*info++ = isc_info_svc_timeout;
 			else
 			{
@@ -1930,7 +1952,7 @@ void Service::query(USHORT			send_item_length,
 		throw;
 	}
 
-	if (!(svc_flags & SVC_thd_running))
+	if (svc_flags & SVC_finished)
 	{
 		if ((svc_flags & SVC_detached) &&
 			svc_trace_manager->needs(TRACE_EVENT_SERVICE_QUERY))
@@ -1939,12 +1961,23 @@ void Service::query(USHORT			send_item_length,
 			svc_trace_manager->event_service_query(&service, send_item_length, send_items,
 				recv_item_length, recv_items, res_successful);
 		}
-
-		finish(SVC_finished);
 	}
 }
 
 
+THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg)
+{
+	Service* svc = (Service*)arg;
+	int exit_code = svc->svc_service_run->serv_thd(svc);
+
+	svc->started();
+	svc->svc_sem_full.release();
+	svc->finish(SVC_finished);
+
+	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
+}
+
+
 void Service::start(USHORT spb_length, const UCHAR* spb_data)
 {
 	ExistenceGuard guard(this, FB_FUNCTION);
@@ -1980,22 +2013,18 @@ void Service::start(USHORT spb_length, const UCHAR* spb_data)
 								Arg::Gds(isc_svc_start_failed));
 	}
 
-	{ // scope for locked globalServicesMutex
+	if (!(svc_flags & SVC_finished)) {
-		MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
+		status_exception::raise(Arg::Gds(isc_svc_in_use) << Arg::Str(serv->serv_name));
-
-		if (svc_flags & SVC_thd_running) {
-			status_exception::raise(Arg::Gds(isc_svc_in_use) << Arg::Str(serv->serv_name));
-		}
-
-		// Another service may have been started with this service block.
-		// If so, we must reset the service flags.
-		svc_switches.erase();
-		if (!(svc_flags & SVC_detached))
-		{
-			svc_flags = 0;
-		}
 	}
 
+	// Another service may have been started with this service block.
+	// If so, we must reset the service flags.
+	svc_switches.erase();
+/*	if (!(svc_flags & SVC_detached))
+	{
+		svc_flags = 0;
+	}
+ */
 	if (!svc_perm_sw.hasData())
 	{
 		// If svc_perm_sw is not used -- call a command-line parsing utility
@@ -2062,7 +2091,6 @@ void Service::start(USHORT spb_length, const UCHAR* spb_data)
        	status_exception::raise(Arg::Gds(isc_adm_task_denied));
     }
 
-
 	// Break up the command line into individual arguments.
 	parseSwitches();
 
@@ -2071,16 +2099,11 @@ void Service::start(USHORT spb_length, const UCHAR* spb_data)
 
 	if (serv->serv_thd)
 	{
-		{	// scope
+		svc_flags &= ~(SVC_evnt_fired | SVC_finished);
-			MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
-			svc_flags &= ~SVC_evnt_fired;
-			svc_flags |= SVC_thd_running;
 
-			svc_stdout_head = 0;
+		svc_stdout_head = svc_stdout_tail = 0;
-			svc_stdout_tail = 0;
-		}
 
-		Thread::start(serv->serv_thd, this, THREAD_medium);
+		Thread::start(run, this, THREAD_medium);
 
 		// Check for the service being detached. This will prevent the thread
 		// from waiting infinitely if the client goes away.
@@ -2092,6 +2115,8 @@ void Service::start(USHORT spb_length, const UCHAR* spb_data)
 			// to include in its status vector information about the service's
 			// ability to start.
 			// This is needed since Thread::start() will almost always succeed.
+			//
+			// Do not unlock mutex here - one can call start not doing this.
 			if (svcStart.tryEnter(60))
 			{
 				// started() was called
@@ -2131,7 +2156,7 @@ void Service::start(USHORT spb_length, const UCHAR* spb_data)
 }
 
 
-THREAD_ENTRY_DECLARE Service::readFbLog(THREAD_ENTRY_PARAM arg)
+int Service::readFbLog(UtilSvc* arg)
 {
 	Service* service = (Service*) arg;
 	service->readFbLog();
@@ -2181,12 +2206,10 @@ void Service::readFbLog()
 	{
 		fclose(file);
 	}
-
-	finish(SVC_finished);
 }
 
 
-void Service::start(ThreadEntryPoint* service_thread)
+void Service::start(const serv_entry* service_run)
 {
 	// Break up the command line into individual arguments.
 	parseSwitches();
@@ -2196,7 +2219,8 @@ void Service::start(ThreadEntryPoint* service_thread)
 		argv[0] = svc_service->serv_name;
 	}
 
-	Thread::start(service_thread, this, THREAD_medium);
+	svc_service_run = service_run;
+	Thread::start(run, this, THREAD_medium);
 }
 
 
@@ -2285,10 +2309,7 @@ void Service::get(UCHAR* buffer, USHORT length, USHORT flags, USHORT timeout, US
 
 	*return_length = 0;
 
-	{	// scope
+	svc_timeout = false;
-		MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
-		svc_flags &= ~SVC_timeout;
-	}
 
 	bool flagFirst = true;
 	while (length)
@@ -2317,7 +2338,10 @@ void Service::get(UCHAR* buffer, USHORT length, USHORT flags, USHORT timeout, US
 				break;
 			}
 
+			UnlockGuard guard(this, FB_FUNCTION);
 			svc_sem_full.tryEnter(1, 0);
+			if (!guard.enter())
+				Arg::Gds(isc_bad_svc_handle).raise();
 		}
 #ifdef HAVE_GETTIMEOFDAY
 		GETTIMEOFDAY(&end_time);
@@ -2328,8 +2352,8 @@ void Service::get(UCHAR* buffer, USHORT length, USHORT flags, USHORT timeout, US
 #endif
 		if (timeout && elapsed_time >= timeout)
 		{
-			MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
+			ExistenceGuard guard(this, FB_FUNCTION);
-			svc_flags |= SVC_timeout;
+			svc_timeout = true;
 			break;
 		}
 
@@ -2458,13 +2482,9 @@ void Service::finish(USHORT flag)
 {
 	if (flag == SVC_finished || flag == SVC_detached)
 	{
-		MutexLockGuard guard(globalServicesMutex, FB_FUNCTION);
+		ExistenceGuard guard(this, FB_FUNCTION);
 
 		svc_flags |= flag;
-		if (! (svc_flags & SVC_thd_running))
-		{
-			svc_flags |= SVC_finished;
-		}
 		if ((svc_flags & SVC_finished) && (svc_flags & SVC_detached))
 		{
 			delete this;
@@ -2490,7 +2510,6 @@ void Service::finish(USHORT flag)
 		if (svc_flags & SVC_finished)
 		{
 			svc_sem_full.release();
-			svc_flags &= ~SVC_thd_running;
 		}
 		else
 		{

src/jrd/svc.h

@@ -36,6 +36,7 @@
 #include "../common/classes/array.h"
 #include "../common/classes/SafeArg.h"
 #include "../common/UtilSvc.h"
+#include "../jrd/EngineInterface.h"
 #include "../common/classes/Switches.h"
 #include "../common/classes/ClumpletReader.h"
 #include "../burp/split/spit.h"
@@ -95,12 +96,12 @@ const USHORT isc_action_max				= isc_action_svc_last;
 //define isc_info_max                  67
 
 // Bitmask values for the svc_flags variable
-const int SVC_shutdown		= 0x1;
+//const int SVC_shutdown	= 0x1;
-const int SVC_timeout		= 0x2;
+//const int SVC_timeout		= 0x2;
 //const int SVC_forked		= 0x4;
 const int SVC_detached		= 0x8;
 const int SVC_finished		= 0x10;
-const int SVC_thd_running	= 0x20;
+//const int SVC_thd_running	= 0x20;
 const int SVC_evnt_fired	= 0x40;
 const int SVC_cmd_line		= 0x80;
 
@@ -124,8 +125,6 @@ public:		// utilities interface with service
 	virtual bool isService();
 	// client thread started
 	virtual void started();
-	// client thread finished
-	virtual void finish();
 	// put various info items in info buffer
     virtual void putLine(char tag, const char* val);
     virtual void putSLong(char tag, SLONG val);
@@ -183,7 +182,7 @@ public:		// external interface with service
 	}
 
 	// Firebird log reader
-	static THREAD_ENTRY_DECLARE readFbLog(THREAD_ENTRY_PARAM arg);
+	static int readFbLog(Firebird::UtilSvc* uSvc);
 	// Shuts all service threads (should be called after databases shutdown)
 	static void shutdownServices();
 
@@ -224,7 +223,7 @@ private:
 	// true if no more space in stdout buffer
 	bool	full() const;
 	// start service thread
-	void	start(ThreadEntryPoint* service_thread);
+	void	start(const serv_entry* service_run);
 	// Set the flag (either SVC_finished for the main service thread or SVC_detached for the client thread).
 	// If both main thread and client thread are completed that is main thread is finished and
 	// client is detached then free memory used by service.
@@ -267,6 +266,8 @@ private:
 	void makePermanentStatusVector() throw();
 	// Read SPB on attach
 	void getOptions(Firebird::ClumpletReader&);
+	// Invoke appropriate service thread entry and finalize it correctly
+	static THREAD_ENTRY_DECLARE run(THREAD_ENTRY_PARAM arg);
 
 private:
 	ISC_STATUS_ARRAY svc_status;		// status vector for running service
@@ -286,6 +287,8 @@ private:
 	USHORT	svc_user_flag;
 	USHORT	svc_spb_version;
 	bool	svc_do_shutdown;
+	bool	svc_shutdown_in_progress;
+	bool	svc_timeout;
 
 	Firebird::string	svc_username;
 	Firebird::AuthReader::AuthBlock	svc_auth_block;
@@ -314,28 +317,44 @@ public:
 
 	Firebird::Semaphore	svc_detach_sem;
 
+	JService* jSvc;
+
 private:
 	StatusStringsHelper	svc_thread_strings;
 
 	Firebird::Semaphore svc_sem_empty, svc_sem_full;
 
+	class SafeMutexLock
+	{
+	public:
+		SafeMutexLock(Service* svc, const char* f);
+		bool lock();
+
+	protected:
+		Firebird::RefPtr<JService> jSvc;
+		const char* from;
+	};
+
+	friend class SafeMutexLock;
+
 	//Service existence guard
-	class ExistenceGuard
+	class ExistenceGuard : private SafeMutexLock
 	{
 	public:
 		explicit ExistenceGuard(Service* svc, const char* from);
 		~ExistenceGuard();
-		void release();
-
-	private:
-		Service* svc;
-		bool locked;
 	};
 
-	friend class ExistenceGuard;
+	//Service unlock guard
-
+	class UnlockGuard : private SafeMutexLock
-	Firebird::Mutex		svc_existence_lock;
+	{
-	ExistenceGuard*		svc_current_guard;
+	public:
+		explicit UnlockGuard(Service* svc, const char* from);
+		bool enter();
+		~UnlockGuard();
+	private:
+		bool locked, doLock;
+	};
 
 	// Data pipe from client to service
 	Firebird::Semaphore svc_stdin_semaphore;

src/jrd/svc_tab.cpp

@@ -35,14 +35,14 @@
 // Service Functions
 #include "../burp/burp_proto.h"
 #include "../alice/alice_proto.h"
-THREAD_ENTRY_DECLARE main_gstat(THREAD_ENTRY_PARAM arg);
+int main_gstat(Firebird::UtilSvc* uSvc);
 #include "../utilities/nbackup/nbk_proto.h"
 #include "../utilities/gsec/gsec_proto.h"
 
 namespace Jrd {
 
 #ifdef DEBUG
-THREAD_ENTRY_DECLARE test_thread(THREAD_ENTRY_PARAM);
+int test_thread(Firebird::UtilSvc* uSvc);
 void test_cmd(USHORT, SCHAR *, TEXT **);
 #define TEST_THREAD test_thread
 #define TEST_CMD test_cmd
@@ -51,7 +51,7 @@ void test_cmd(USHORT, SCHAR *, TEXT **);
 // removed as the services API takes shape.  They are used to
 // test that the paths for starting services and parsing command-lines
 // are followed correctly.
-THREAD_ENTRY_DECLARE test_thread(THREAD_ENTRY_PARAM)
+int test_thread(Firebird::UtilSvc* uSvc)
 {
 	gds__log("Starting service");
 	return FINI_OK;

src/jrd/svc_tab.h

@@ -29,16 +29,18 @@
 #ifndef JRD_SVC_TAB_H
 #define JRD_SVC_TAB_H
 
-#include "../common/ThreadStart.h"
+#include "../common/UtilSvc.h"
 
 namespace Jrd {
 
+typedef int ServiceEntry(Firebird::UtilSvc*);
+
 struct serv_entry
 {
 	USHORT				serv_action;		// isc_action_svc_....
 	const TEXT*			serv_name;			// old service name
 	const TEXT*			serv_std_switches;	// old cmd-line switches
-	ThreadEntryPoint*	serv_thd;			// thread to execute
+	ServiceEntry*		serv_thd;			// thread to execute
 };
 
 extern const serv_entry services[];

src/jrd/trace/TraceService.cpp

@@ -331,7 +331,7 @@ bool TraceSvcJrd::checkAliveAndFlags(ULONG sesId, int& flags)
 
 
 // service entrypoint
-THREAD_ENTRY_DECLARE TRACE_main(THREAD_ENTRY_PARAM arg)
+int TRACE_main(UtilSvc* arg)
 {
 	Service* svc = (Service*) arg;
 	int exit_code = FB_SUCCESS;
@@ -350,8 +350,5 @@ THREAD_ENTRY_DECLARE TRACE_main(THREAD_ENTRY_PARAM arg)
 		exit_code = FB_FAILURE;
 	}
 
-	svc->started();
+	return exit_code;
-	svc->finish();
-
-	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
 }

src/jrd/trace/TraceService.h

@@ -39,7 +39,7 @@
 #include "../../jrd/trace/TraceSession.h"
 
 
-THREAD_ENTRY_DECLARE TRACE_main(THREAD_ENTRY_PARAM);
+int TRACE_main(Firebird::UtilSvc*);
 
 
 namespace Firebird {

src/utilities/gsec/gsec.cpp

@@ -72,7 +72,7 @@ static void insert_error(ISC_STATUS*, ISC_STATUS);
 static void msg_get(USHORT number, TEXT* msg);
 
 
-THREAD_ENTRY_DECLARE GSEC_main(THREAD_ENTRY_PARAM arg)
+int GSEC_main(Firebird::UtilSvc* uSvc)
 {
 /**********************************************
  *
@@ -82,7 +82,6 @@ THREAD_ENTRY_DECLARE GSEC_main(THREAD_ENTRY_PARAM arg)
  * Functional Description:
  *   Entrypoint for GSEC via the services manager
  **********************************************/
-	Firebird::UtilSvc* uSvc = (Firebird::UtilSvc*) arg;
 	int exit_code = FINI_OK;
 
 	try {
@@ -97,8 +96,7 @@ THREAD_ENTRY_DECLARE GSEC_main(THREAD_ENTRY_PARAM arg)
 		exit_code = FB_FAILURE;
 	}
 
-	uSvc->finish();
+	return exit_code;
-	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
 }
 
 

src/utilities/gsec/gsec_proto.h

@@ -15,6 +15,6 @@ void	GSEC_print_partial(USHORT);
 void	GSEC_diag(USHORT);
 
 int		gsec(Firebird::UtilSvc*);
-THREAD_ENTRY_DECLARE GSEC_main(THREAD_ENTRY_PARAM);
+int		GSEC_main(Firebird::UtilSvc*);
 
 #endif // GSEC_PROTO_H

src/utilities/gstat/dba.epp

@@ -313,7 +313,7 @@ namespace
 const USHORT GSTAT_MSG_FAC	= 21;
 
 
-THREAD_ENTRY_DECLARE main_gstat(THREAD_ENTRY_PARAM arg)
+int main_gstat(Firebird::UtilSvc* uSvc)
 {
 /**********************************************
  *
@@ -323,7 +323,6 @@ THREAD_ENTRY_DECLARE main_gstat(THREAD_ENTRY_PARAM arg)
  * Functional Description:
  *   Entrypoint for GSTAT via the services manager
  **********************************************/
-	Firebird::UtilSvc* uSvc = (Firebird::UtilSvc*) arg;
 	int exit_code = FINI_OK;
 
 	try {
@@ -338,8 +337,7 @@ THREAD_ENTRY_DECLARE main_gstat(THREAD_ENTRY_PARAM arg)
 		exit_code = FB_FAILURE;
 	}
 
-	uSvc->finish();
+	return exit_code;
-	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
 }
 
 

src/utilities/nbackup/nbackup.cpp

@@ -1402,9 +1402,8 @@ void NBackup::restore_database(const BackupFiles& files)
 	}
 }
 
-THREAD_ENTRY_DECLARE NBACKUP_main(THREAD_ENTRY_PARAM arg)
+int NBACKUP_main(UtilSvc* uSvc)
 {
-	UtilSvc* uSvc = (UtilSvc*) arg;
 	int exit_code = FB_SUCCESS;
 
 	try {
@@ -1435,9 +1434,7 @@ THREAD_ENTRY_DECLARE NBACKUP_main(THREAD_ENTRY_PARAM arg)
 		exit_code = FB_FAILURE;
 	}
 
-	uSvc->started();
+	return exit_code;
-	uSvc->finish();
-	return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
 }
 
 enum NbOperation {nbNone, nbLock, nbUnlock, nbFixup, nbBackup, nbRestore};

src/utilities/nbackup/nbk_proto.h

@@ -31,6 +31,6 @@
 #include "../common/UtilSvc.h"
 
 void nbackup(Firebird::UtilSvc*);
-THREAD_ENTRY_DECLARE NBACKUP_main(THREAD_ENTRY_PARAM);
+int NBACKUP_main(Firebird::UtilSvc*);
 
 #endif // NBK_PROTO_H