Fixed running as a service on Windows NT family.

cmd/synergyc/synergyc.cpp

@@ -145,6 +145,7 @@ realMain(void)
 				locked = false;
 				s_client->mainLoop();
 				locked = true;
+				DAEMON_RUNNING(false);
 
 				// get client status
 				if (s_client->wasRejected()) {
@@ -157,6 +158,7 @@ realMain(void)
 #define FINALLY do {								\
 				if (!locked) {						\
 					DAEMON_RUNNING(false);			\
+					locked = true;					\
 				}									\
 				if (s_client != NULL) {				\
 					if (opened) {					\
@@ -488,13 +490,6 @@ byeThrow(int x)
 	CArchMiscWindows::daemonFailed(x);
 }
 
-static
-void
-daemonStop(void)
-{
-	s_client->exitMainLoop();
-}
-
 static
 int
 daemonStartup(int argc, const char** argv)
@@ -511,7 +506,7 @@ daemonStartup(int argc, const char** argv)
 	ARG->m_backend = false;
 
 	// run as a service
-	return CArchMiscWindows::runDaemon(realMain, daemonStop);
+	return CArchMiscWindows::runDaemon(realMain);
 }
 
 static
@@ -545,12 +540,13 @@ WinMain(HINSTANCE instance, HINSTANCE, LPSTR, int)
 	// users on NT can use `--daemon' or `--no-daemon' to force us out
 	// of the service code path.
 	if (__argc <= 1 && !CArchMiscWindows::isWindows95Family()) {
-		int result = ARCH->daemonize(DAEMON_NAME, &daemonStartup);
+		try {
-		if (result == -1) {
+			return ARCH->daemonize(DAEMON_NAME, &daemonStartup);
-			LOG((CLOG_CRIT "failed to start as a service" BYE, ARG->m_pname));
+		}
+		catch (XArchDaemon& e) {
+			LOG((CLOG_CRIT "failed to start as a service: %s" BYE, e.what().c_str(), ARG->m_pname));
 			return kExitFailed;
 		}
-		return result;
 	}
 
 	// parse command line
@@ -564,8 +560,8 @@ WinMain(HINSTANCE instance, HINSTANCE, LPSTR, int)
 			try {
 				result = ARCH->daemonize(DAEMON_NAME, &daemonStartup95);
 			}
-			catch (XArchDaemon&) {
+			catch (XArchDaemon& e) {
-				LOG((CLOG_CRIT "failed to start as a service" BYE, ARG->m_pname));
+				LOG((CLOG_CRIT "failed to start as a service: %s" BYE, e.what().c_str(), ARG->m_pname));
 				result = kExitFailed;
 			}
 		}

cmd/synergys/synergys.cpp

@@ -175,12 +175,12 @@ realMain(void)
 				DAEMON_RUNNING(true);
 				locked = false;
 				s_server->mainLoop();
-				locked = true;
 
 				// clean up
 #define FINALLY do {								\
 				if (!locked) {						\
 					DAEMON_RUNNING(false);			\
+					locked = true;					\
 				}									\
 				if (s_server != NULL) {				\
 					if (opened) {					\
@@ -209,7 +209,7 @@ realMain(void)
 			catch (...) {
 				// don't try to restart and fail
 				ARG->m_restartable = false;
-				result           = kExitFailed;
+				result             = kExitFailed;
 				FINALLY;
 			}
 #undef FINALLY
@@ -220,7 +220,7 @@ realMain(void)
 		catch (XThread&) {
 			// terminated
 			ARG->m_restartable = false;
-			result           = kExitTerminated;
+			result             = kExitTerminated;
 		}
 	} while (ARG->m_restartable);
 
@@ -619,13 +619,6 @@ byeThrow(int x)
 	CArchMiscWindows::daemonFailed(x);
 }
 
-static
-void
-daemonStop(void)
-{
-	s_server->exitMainLoop();
-}
-
 static
 int
 daemonStartup(int argc, const char** argv)
@@ -645,7 +638,7 @@ daemonStartup(int argc, const char** argv)
 	loadConfig();
 
 	// run as a service
-	return CArchMiscWindows::runDaemon(realMain, daemonStop);
+	return CArchMiscWindows::runDaemon(realMain);
 }
 
 static
@@ -682,8 +675,8 @@ WinMain(HINSTANCE instance, HINSTANCE, LPSTR, int)
 		try {
 			return ARCH->daemonize(DAEMON_NAME, &daemonStartup);
 		}
-		catch (XArchDaemon&) {
+		catch (XArchDaemon& e) {
-			LOG((CLOG_CRIT "failed to start as a service" BYE, ARG->m_pname));
+			LOG((CLOG_CRIT "failed to start as a service: %s" BYE, e.what().c_str(), ARG->m_pname));
 			return kExitFailed;
 		}
 	}
@@ -702,8 +695,8 @@ WinMain(HINSTANCE instance, HINSTANCE, LPSTR, int)
 			try {
 				result = ARCH->daemonize(DAEMON_NAME, &daemonStartup95);
 			}
-			catch (XArchDaemon&) {
+			catch (XArchDaemon& e) {
-				LOG((CLOG_CRIT "failed to start as a service" BYE, ARG->m_pname));
+				LOG((CLOG_CRIT "failed to start as a service: %s" BYE, e.what().c_str(), ARG->m_pname));
 				result = kExitFailed;
 			}
 		}

lib/arch/CArchDaemonWindows.cpp

@@ -24,7 +24,8 @@
 
 CArchDaemonWindows*		CArchDaemonWindows::s_daemon = NULL;
 
-CArchDaemonWindows::CArchDaemonWindows()
+CArchDaemonWindows::CArchDaemonWindows() :
+	m_daemonThread(NULL)
 {
 	// do nothing
 }
@@ -35,11 +36,11 @@ CArchDaemonWindows::~CArchDaemonWindows()
 }
 
 int
-CArchDaemonWindows::runDaemon(RunFunc runFunc, StopFunc stopFunc)
+CArchDaemonWindows::runDaemon(RunFunc runFunc)
 {
 	assert(s_daemon != NULL);
 
-	return s_daemon->doRunDaemon(runFunc, stopFunc);
+	return s_daemon->doRunDaemon(runFunc);
 }
 
 void
@@ -293,7 +294,7 @@ CArchDaemonWindows::daemonize(const char* name, DaemonFunc func)
 		// hook us up to the service control manager.  this won't return
 		// (if successful) until the processes have terminated.
 		s_daemon = this;
-		if (StartServiceCtrlDispatcher(entry)) {
+		if (StartServiceCtrlDispatcher(entry) == 0) {
 			// StartServiceCtrlDispatcher failed
 			s_daemon = NULL;
 			throw XArchDaemonFailed(new XArchEvalWindows);
@@ -536,7 +537,7 @@ CArchDaemonWindows::openUserStartupKey()
 }
 
 int
-CArchDaemonWindows::doRunDaemon(RunFunc run, StopFunc stop)
+CArchDaemonWindows::doRunDaemon(RunFunc run)
 {
 	// should only be called from DaemonFunc
 	assert(m_serviceMutex != NULL);
@@ -546,7 +547,6 @@ CArchDaemonWindows::doRunDaemon(RunFunc run, StopFunc stop)
 	ARCH->lockMutex(m_serviceMutex);
 	try {
 		int result;
-		m_stop                  = stop;
 		m_serviceHandlerWaiting = false;
 		m_serviceRunning        = false;
 		for (;;) {
@@ -555,13 +555,11 @@ CArchDaemonWindows::doRunDaemon(RunFunc run, StopFunc stop)
 
 			// run callback in another thread
 			m_serviceRunning = true;
-			{
+			m_daemonThread = ARCH->newThread(
-				CArchThread thread = ARCH->newThread(
 							&CArchDaemonWindows::runDaemonThreadEntry, run);
-				ARCH->wait(thread, -1.0);
+			ARCH->wait(m_daemonThread, -1.0);
-				result = reinterpret_cast<int>(ARCH->getResultOfThread(thread));
+			result = reinterpret_cast<int>(
-				ARCH->closeThread(thread);
+							ARCH->getResultOfThread(m_daemonThread));
-			}
 			m_serviceRunning = false;
 
 			// notify handler that the server stopped.  if handler
@@ -585,6 +583,10 @@ CArchDaemonWindows::doRunDaemon(RunFunc run, StopFunc stop)
 			if (m_serviceState == SERVICE_STOPPED) {
 				break;
 			}
+
+			// done with callback thread
+			ARCH->closeThread(m_daemonThread);
+			m_daemonThread = NULL;
 		}
 
 		// prevent daemonHandler from changing state
@@ -598,7 +600,10 @@ CArchDaemonWindows::doRunDaemon(RunFunc run, StopFunc stop)
 		setStatus(m_serviceState);
 
 		// clean up
-		m_stop = NULL;
+		if (m_daemonThread != NULL) {
+			ARCH->closeThread(m_daemonThread);
+			m_daemonThread = NULL;
+		}
 		ARCH->unlockMutex(m_serviceMutex);
 
 		return result;
@@ -716,7 +721,7 @@ CArchDaemonWindows::serviceMain(DWORD argc, LPTSTR* argvIn)
 	}
 
 	// tell service control manager that we're starting
-	setStatus(SERVICE_START_PENDING, 0, 1000);
+	setStatus(SERVICE_START_PENDING, 0, 10000);
 
 	// if no arguments supplied then try getting them from the registry.
 	// the first argument doesn't count because it's the service name.
@@ -830,12 +835,12 @@ CArchDaemonWindows::serviceHandler(DWORD ctrl)
 	case SERVICE_CONTROL_PAUSE:
 		// update state
 		m_serviceState = SERVICE_PAUSE_PENDING;
-		setStatus(m_serviceState, 0, 1000);
+		setStatus(m_serviceState, 0, 5000);
 
 		// stop run callback if running and wait for it to finish
 		if (m_serviceRunning) {
 			m_serviceHandlerWaiting = true;
-			m_stop();
+			ARCH->cancelThread(m_daemonThread);
 			ARCH->waitCondVar(m_serviceCondVar, m_serviceMutex, -1.0);
 		}
 
@@ -860,12 +865,12 @@ CArchDaemonWindows::serviceHandler(DWORD ctrl)
 	case SERVICE_CONTROL_SHUTDOWN:
 		// update state
 		m_serviceState = SERVICE_STOP_PENDING;
-		setStatus(m_serviceState, 0, 1000);
+		setStatus(m_serviceState, 0, 5000);
 
 		// stop run callback if running and wait for it to finish
 		if (m_serviceRunning) {
 			m_serviceHandlerWaiting = true;
-			m_stop();
+			ARCH->cancelThread(m_daemonThread);
 			ARCH->waitCondVar(m_serviceCondVar, m_serviceMutex, -1.0);
 		}
 

lib/arch/CArchDaemonWindows.h

@@ -29,7 +29,6 @@
 class CArchDaemonWindows : public IArchDaemon {
 public:
 	typedef int			(*RunFunc)(void);
-	typedef void		(*StopFunc)(void);
 
 	CArchDaemonWindows();
 	virtual ~CArchDaemonWindows();
@@ -41,12 +40,13 @@ public:
 	daemon processing.  The \c runFunc should perform the daemon's
 	main loop, calling \c daemonRunning(true) when it enters the main loop
 	(i.e. after initialization) and \c daemonRunning(false) when it leaves
-	the main loop.  The \c stopFunc function is called when the daemon
+	the main loop.  The \c runFunc is called in a new thread and when the
-	must exit the main loop and must cause \c runFunc to return.  This
+	daemon must exit the main loop due to some external control the
-	function returns what \c runFunc returns.  \c runFunc should call
+	thread is cancelled on behalf of the client.  This function returns
-	\c daemonFailed() if the daemon fails.
+	what \c runFunc returns.  \c runFunc should call \c daemonFailed() if
+	the daemon fails.
 	*/
-	static int			runDaemon(RunFunc runFunc, StopFunc stopFunc);
+	static int			runDaemon(RunFunc runFunc);
 
 	//! Indicate daemon is in main loop
 	/*!
@@ -87,7 +87,7 @@ private:
 	static HKEY			open95ServicesKey();
 	static HKEY			openUserStartupKey();
 
-	int					doRunDaemon(RunFunc runFunc, StopFunc stopFunc);
+	int					doRunDaemon(RunFunc runFunc);
 	void				doDaemonRunning(bool running);
 
 	static void			setStatus(DWORD state);
@@ -120,8 +120,8 @@ private:
 	DWORD				m_serviceState;
 	bool				m_serviceHandlerWaiting;
 	bool				m_serviceRunning;
-	StopFunc			m_stop;
 
+	CArchThread			m_daemonThread;
 	DaemonFunc			m_daemonFunc;
 	int					m_daemonResult;
 

lib/arch/CArchLogWindows.cpp

@@ -77,7 +77,7 @@ CArchLogWindows::writeLog(ELevel level, const char* msg)
 								0,					// event ID
 								NULL,
 								0,
-								strlen(msg + 1),	// raw data size
+								strlen(msg) + 1,	// raw data size
 								NULL,
 								const_cast<char*>(msg));// raw data
 	}

lib/arch/CArchMiscWindows.cpp

@@ -35,9 +35,9 @@ CArchMiscWindows::isWindows95Family()
 }
 
 int
-CArchMiscWindows::runDaemon(RunFunc runFunc, StopFunc stopFunc)
+CArchMiscWindows::runDaemon(RunFunc runFunc)
 {
-	return CArchDaemonWindows::runDaemon(runFunc, stopFunc);
+	return CArchDaemonWindows::runDaemon(runFunc);
 }
 
 void

lib/arch/CArchMiscWindows.h

@@ -19,7 +19,6 @@
 class CArchMiscWindows {
 public:
 	typedef int			(*RunFunc)(void);
-	typedef void		(*StopFunc)(void);
 
 	//! Test if windows 95, et al.
 	/*!
@@ -31,7 +30,7 @@ public:
 	/*!
 	Delegates to CArchDaemonWindows.
 	*/
-	static int			runDaemon(RunFunc runFunc, StopFunc stopFunc);
+	static int			runDaemon(RunFunc runFunc);
 
 	//! Indicate daemon is in main loop
 	/*!

lib/arch/CArchMultithreadWindows.cpp

@@ -311,7 +311,7 @@ CArchMultithreadWindows::closeThread(CArchThread thread)
 
 		// remove thread from list
 		lockMutex(m_threadMutex);
-		assert(findNoRef(thread->m_id) == thread);
+		assert(findNoRefOrCreate(thread->m_id) == thread);
 		erase(thread);
 		unlockMutex(m_threadMutex);
 
@@ -531,6 +531,23 @@ CArchMultithreadWindows::find(DWORD id)
 
 CArchThreadImpl*
 CArchMultithreadWindows::findNoRef(DWORD id)
+{
+	CArchThreadImpl* impl = findNoRefOrCreate(id);
+	if (impl == NULL) {
+		// create thread for calling thread which isn't in our list and
+		// add it to the list.  this won't normally happen but it can if
+		// the system calls us under a new thread, like it does when we
+		// run as a service.
+		impl           = new CArchThreadImpl;
+		impl->m_thread = NULL;
+		impl->m_id     = GetCurrentThreadId();
+		insert(impl);
+	}
+	return impl;
+}
+
+CArchThreadImpl*
+CArchMultithreadWindows::findNoRefOrCreate(DWORD id)
 {
 	// linear search
 	for (CThreadList::const_iterator index  = m_threadList.begin();
@@ -548,7 +565,7 @@ CArchMultithreadWindows::insert(CArchThreadImpl* thread)
 	assert(thread != NULL);
 
 	// thread shouldn't already be on the list
-	assert(findNoRef(thread->m_id) == NULL);
+	assert(findNoRefOrCreate(thread->m_id) == NULL);
 
 	// append to list
 	m_threadList.push_back(thread);
@@ -570,7 +587,7 @@ void
 CArchMultithreadWindows::refThread(CArchThreadImpl* thread)
 {
 	assert(thread != NULL);
-	assert(findNoRef(thread->m_id) != NULL);
+	assert(findNoRefOrCreate(thread->m_id) != NULL);
 	++thread->m_refCount;
 }
 

lib/arch/CArchMultithreadWindows.h

@@ -78,6 +78,7 @@ public:
 private:
 	CArchThreadImpl*	find(DWORD id);
 	CArchThreadImpl*	findNoRef(DWORD id);
+	CArchThreadImpl*	findNoRefOrCreate(DWORD id);
 	void				insert(CArchThreadImpl* thread);
 	void				erase(CArchThreadImpl* thread);