From 1cbdaee31b8af80c334670bff11cc6a88f0f800d Mon Sep 17 00:00:00 2001 From: crs Date: Mon, 3 Jun 2002 13:45:30 +0000 Subject: [PATCH] added better handling of X server disconnecting unexpectedly. the apps still exit but they do it in a mostly controlled manner. in particular, the server threads except the one processing primary screen events will terminate gracefully. this will be important should the server ever allow HTTP clients to rewrite the configuration file. note that X makes it effectively impossible to continue once the X server disconnects. even if it didn't it would be difficult for synergy to recover. users will have to add synergy to the X display manager's startup script if they expect the server to be restarted. alternatively, we could add code to fork synergy at startup; the child would do the normal work while the parent would simply wait for the child to exit and restart it. --- client/CClient.cpp | 20 ++++- client/CXWindowsSecondaryScreen.cpp | 19 ++--- client/CXWindowsSecondaryScreen.h | 4 +- server/CServer.cpp | 121 +++++++++++++++++++++------- server/CServer.h | 8 +- server/CXWindowsPrimaryScreen.cpp | 26 +++--- server/CXWindowsPrimaryScreen.h | 5 +- synergy/CXWindowsScreen.cpp | 52 +++++++++--- synergy/CXWindowsScreen.h | 24 ++++-- synergy/Makefile | 1 + synergy/XScreen.cpp | 10 +++ synergy/XScreen.h | 14 ++++ synergy/synergy.dsp | 8 ++ 13 files changed, 242 insertions(+), 70 deletions(-) create mode 100644 synergy/XScreen.cpp create mode 100644 synergy/XScreen.h diff --git a/client/CClient.cpp b/client/CClient.cpp index de4e7de0..a5178e43 100644 --- a/client/CClient.cpp +++ b/client/CClient.cpp @@ -10,6 +10,7 @@ #include "CThread.h" #include "CTimerThread.h" #include "TMethodJob.h" +#include "XScreen.h" #include "XSynergy.h" #include "XThread.h" #include @@ -51,7 +52,16 @@ void CClient::run(const CNetworkAddress& serverAddress) log((CLOG_NOTE "starting client")); // connect to secondary screen - openSecondaryScreen(); + while (m_screen == NULL) { + try { + openSecondaryScreen(); + } + catch (XScreenOpenFailure&) { + // can't open screen yet. wait a few seconds to retry. + log((CLOG_INFO "failed to open screen. waiting to retry.")); + CThread::sleep(3.0); + } + } // start server interactions m_serverAddress = &serverAddress; @@ -88,7 +98,9 @@ void CClient::run(const CNetworkAddress& serverAddress) thread->wait(); delete thread; } - closeSecondaryScreen(); + if (m_screen != NULL) { + closeSecondaryScreen(); + } throw; } catch (...) { @@ -101,7 +113,9 @@ void CClient::run(const CNetworkAddress& serverAddress) thread->wait(); delete thread; } - closeSecondaryScreen(); + if (m_screen != NULL) { + closeSecondaryScreen(); + } throw; } } diff --git a/client/CXWindowsSecondaryScreen.cpp b/client/CXWindowsSecondaryScreen.cpp index 68672a7e..e3e3aa97 100644 --- a/client/CXWindowsSecondaryScreen.cpp +++ b/client/CXWindowsSecondaryScreen.cpp @@ -290,12 +290,11 @@ void CXWindowsSecondaryScreen::getClipboard( getDisplayClipboard(id, clipboard); } -void CXWindowsSecondaryScreen::onOpenDisplay() +void CXWindowsSecondaryScreen::onOpenDisplay( + Display* display) { assert(m_window == None); - CDisplayLock display(this); - // create the cursor hiding window. this window is used to hide the // cursor when it's not on the screen. the window is hidden as soon // as the cursor enters the screen or the display's real cursor is @@ -326,16 +325,18 @@ CXWindowsClipboard* CXWindowsSecondaryScreen::createClipboard( return new CXWindowsClipboard(display, m_window, id); } -void CXWindowsSecondaryScreen::onCloseDisplay() +void CXWindowsSecondaryScreen::onCloseDisplay( + Display* display) { assert(m_window != None); - // no longer impervious to server grabs - CDisplayLock display(this); - XTestGrabControl(display, False); + if (display != NULL) { + // no longer impervious to server grabs + XTestGrabControl(display, False); - // destroy window - XDestroyWindow(display, m_window); + // destroy window + XDestroyWindow(display, m_window); + } m_window = None; } diff --git a/client/CXWindowsSecondaryScreen.h b/client/CXWindowsSecondaryScreen.h index 2617096c..d4a00a48 100644 --- a/client/CXWindowsSecondaryScreen.h +++ b/client/CXWindowsSecondaryScreen.h @@ -35,10 +35,10 @@ public: protected: // CXWindowsScreen overrides - virtual void onOpenDisplay(); + virtual void onOpenDisplay(Display*); virtual CXWindowsClipboard* createClipboard(ClipboardID); - virtual void onCloseDisplay(); + virtual void onCloseDisplay(Display*); virtual void onLostClipboard(ClipboardID); private: diff --git a/server/CServer.cpp b/server/CServer.cpp index 9b6686d9..02ed0bd1 100644 --- a/server/CServer.cpp +++ b/server/CServer.cpp @@ -17,6 +17,7 @@ #include "CStopwatch.h" #include "CFunctionJob.h" #include "TMethodJob.h" +#include "XScreen.h" #include "XSocket.h" #include "XSynergy.h" #include "XThread.h" @@ -46,7 +47,8 @@ else { wait(0); exit(1); } const SInt32 CServer::s_httpMaxSimultaneousRequests = 3; -CServer::CServer() : m_primary(NULL), +CServer::CServer() : m_cleanupSize(&m_mutex, 0), + m_primary(NULL), m_active(NULL), m_primaryInfo(NULL), m_seqNum(0), @@ -69,7 +71,16 @@ void CServer::run() log((CLOG_NOTE "starting server")); // connect to primary screen - openPrimaryScreen(); + while (m_primary == NULL) { + try { + openPrimaryScreen(); + } + catch (XScreenOpenFailure&) { + // can't open screen yet. wait a few seconds to retry. + log((CLOG_INFO "failed to open screen. waiting to retry.")); + CThread::sleep(3.0); + } + } // start listening for HTTP requests m_httpServer = new CHTTPServer(this); @@ -84,9 +95,9 @@ void CServer::run() // clean up log((CLOG_NOTE "stopping server")); + cleanupThreads(); delete m_httpServer; m_httpServer = NULL; - cleanupThreads(); closePrimaryScreen(); } catch (XBase& e) { @@ -94,18 +105,20 @@ void CServer::run() // clean up log((CLOG_NOTE "stopping server")); + cleanupThreads(); delete m_httpServer; m_httpServer = NULL; - cleanupThreads(); closePrimaryScreen(); } catch (XThread&) { // clean up log((CLOG_NOTE "stopping server")); + cleanupThreads(); delete m_httpServer; m_httpServer = NULL; - cleanupThreads(); - closePrimaryScreen(); + if (m_primary != NULL) { + closePrimaryScreen(); + } throw; } catch (...) { @@ -113,10 +126,12 @@ void CServer::run() // clean up log((CLOG_NOTE "stopping server")); + cleanupThreads(); delete m_httpServer; m_httpServer = NULL; - cleanupThreads(); - closePrimaryScreen(); + if (m_primary != NULL) { + closePrimaryScreen(); + } throw; } } @@ -126,6 +141,19 @@ void CServer::quit() m_primary->stop(); } +void CServer::shutdown() +{ + // stop all running threads but don't wait too long since some + // threads may be unable to proceed until this thread returns. + cleanupThreads(3.0); + + // done with the HTTP server + delete m_httpServer; + m_httpServer = NULL; + + // note -- we do not attempt to close down the primary screen +} + bool CServer::setConfig(const CConfig& config) { typedef std::vector CThreads; @@ -1289,19 +1317,29 @@ void CServer::openPrimaryScreen() // reset sequence number m_seqNum = 0; - // add connection - m_active = addConnection(CString("primary"/* FIXME */), NULL); - m_primaryInfo = m_active; + try { + // add connection + m_active = addConnection(CString("primary"/* FIXME */), NULL); + m_primaryInfo = m_active; - // open screen - log((CLOG_DEBUG1 "creating primary screen")); + // open screen + log((CLOG_DEBUG1 "creating primary screen")); #if defined(CONFIG_PLATFORM_WIN32) - m_primary = new CMSWindowsPrimaryScreen; + m_primary = new CMSWindowsPrimaryScreen; #elif defined(CONFIG_PLATFORM_UNIX) - m_primary = new CXWindowsPrimaryScreen; + m_primary = new CXWindowsPrimaryScreen; #endif - log((CLOG_DEBUG1 "opening primary screen")); - m_primary->open(this); + log((CLOG_DEBUG1 "opening primary screen")); + m_primary->open(this); + } + catch (...) { + delete m_primary; + removeConnection(CString("primary"/* FIXME */)); + m_primary = NULL; + m_primaryInfo = NULL; + m_active = NULL; + throw; + } // set the clipboard owner to the primary screen and then get the // current clipboard data. @@ -1340,6 +1378,7 @@ void CServer::addCleanupThread(const CThread& thread) { CLock lock(&m_mutex); m_cleanupList.insert(m_cleanupList.begin(), new CThread(thread)); + m_cleanupSize = m_cleanupSize + 1; } void CServer::removeCleanupThread(const CThread& thread) @@ -1350,30 +1389,52 @@ void CServer::removeCleanupThread(const CThread& thread) if (**index == thread) { CThread* thread = *index; m_cleanupList.erase(index); + m_cleanupSize = m_cleanupSize - 1; + if (m_cleanupSize == 0) { + m_cleanupSize.broadcast(); + } delete thread; return; } } } -void CServer::cleanupThreads() +void CServer::cleanupThreads(double timeout) { log((CLOG_DEBUG1 "cleaning up threads")); - m_mutex.lock(); - while (m_cleanupList.begin() != m_cleanupList.end()) { - // get the next thread and cancel it - CThread* thread = m_cleanupList.front(); - thread->cancel(); - // wait for thread to finish with cleanup list unlocked. the - // thread will remove itself from the cleanup list. - m_mutex.unlock(); - thread->wait(); - m_mutex.lock(); + // first cancel every thread except the current one (with mutex + // locked so the cleanup list won't change). + CLock lock(&m_mutex); + CThread current(CThread::getCurrentThread()); + SInt32 minCount = 0; + for (CThreadList::iterator index = m_cleanupList.begin(); + index != m_cleanupList.end(); ++index) { + CThread* thread = *index; + if (thread != ¤t) { + thread->cancel(); + } + else { + minCount = 1; + } } - // FIXME -- delete remaining threads from list - m_mutex.unlock(); + // now wait for the threads (with mutex unlocked as each thread + // will remove itself from the list) + CStopwatch timer(true); + while (m_cleanupSize > minCount) { + m_cleanupSize.wait(timer, timeout); + } + + // delete remaining threads + for (CThreadList::iterator index = m_cleanupList.begin(); + index != m_cleanupList.end(); ++index) { + CThread* thread = *index; + delete thread; + } + m_cleanupList.clear(); + m_cleanupSize = 0; + log((CLOG_DEBUG1 "cleaned up threads")); } diff --git a/server/CServer.h b/server/CServer.h index d05bc867..37b1eb8f 100644 --- a/server/CServer.h +++ b/server/CServer.h @@ -34,6 +34,11 @@ public: // tell server to exit gracefully void quit(); + // tell the server to shutdown. this is called in an emergency + // when we need to tell the server that we cannot continue. the + // server will attempt to clean up. + void shutdown(); + // update screen map. returns true iff the new configuration was // accepted. bool setConfig(const CConfig&); @@ -173,7 +178,7 @@ private: void updatePrimaryClipboard(ClipboardID); // cancel running threads - void cleanupThreads(); + void cleanupThreads(double timeout = -1.0); // thread method to accept incoming client connections void acceptClients(void*); @@ -220,6 +225,7 @@ private: ISecurityFactory* m_securityFactory; CThreadList m_cleanupList; + CCondVar m_cleanupSize; IPrimaryScreen* m_primary; CScreenList m_screens; diff --git a/server/CXWindowsPrimaryScreen.cpp b/server/CXWindowsPrimaryScreen.cpp index 9d5a828b..9fe5d801 100644 --- a/server/CXWindowsPrimaryScreen.cpp +++ b/server/CXWindowsPrimaryScreen.cpp @@ -179,6 +179,7 @@ void CXWindowsPrimaryScreen::run() void CXWindowsPrimaryScreen::stop() { + CDisplayLock display(this); doStop(); } @@ -187,12 +188,12 @@ void CXWindowsPrimaryScreen::open(CServer* server) assert(m_server == NULL); assert(server != NULL); - // set the server - m_server = server; - // open the display openDisplay(); + // set the server + m_server = server; + // check for peculiarities // FIXME -- may have to get these from some database m_numLockHalfDuplex = false; @@ -419,12 +420,10 @@ bool CXWindowsPrimaryScreen::isLockedToScreen() const return false; } -void CXWindowsPrimaryScreen::onOpenDisplay() +void CXWindowsPrimaryScreen::onOpenDisplay(Display* display) { assert(m_window == None); - CDisplayLock display(this); - // get size of screen SInt32 w, h; getScreenSize(&w, &h); @@ -458,16 +457,25 @@ CXWindowsClipboard* CXWindowsPrimaryScreen::createClipboard( return new CXWindowsClipboard(display, m_window, id); } -void CXWindowsPrimaryScreen::onCloseDisplay() +void CXWindowsPrimaryScreen::onCloseDisplay(Display* display) { assert(m_window != None); // destroy window - CDisplayLock display(this); - XDestroyWindow(display, m_window); + if (display != NULL) { + XDestroyWindow(display, m_window); + } m_window = None; } +void CXWindowsPrimaryScreen::onUnexpectedClose() +{ + // tell server to shutdown + if (m_server != NULL) { + m_server->shutdown(); + } +} + void CXWindowsPrimaryScreen::onLostClipboard( ClipboardID id) { diff --git a/server/CXWindowsPrimaryScreen.h b/server/CXWindowsPrimaryScreen.h index 05c53a30..0590deaa 100644 --- a/server/CXWindowsPrimaryScreen.h +++ b/server/CXWindowsPrimaryScreen.h @@ -30,10 +30,11 @@ public: protected: // CXWindowsScreen overrides - virtual void onOpenDisplay(); + virtual void onOpenDisplay(Display*); virtual CXWindowsClipboard* createClipboard(ClipboardID); - virtual void onCloseDisplay(); + virtual void onCloseDisplay(Display*); + virtual void onUnexpectedClose(); virtual void onLostClipboard(ClipboardID); private: diff --git a/synergy/CXWindowsScreen.cpp b/synergy/CXWindowsScreen.cpp index 5b73e9fc..25602a4a 100644 --- a/synergy/CXWindowsScreen.cpp +++ b/synergy/CXWindowsScreen.cpp @@ -6,6 +6,8 @@ #include "CLog.h" #include "CString.h" #include "CThread.h" +#include "XScreen.h" +#include #include #include @@ -13,29 +15,38 @@ // CXWindowsScreen // +CXWindowsScreen* CXWindowsScreen::s_screen = NULL; + CXWindowsScreen::CXWindowsScreen() : m_display(NULL), m_root(None), m_w(0), m_h(0), m_stop(false) { - // do nothing + assert(s_screen == NULL); + s_screen = this; } CXWindowsScreen::~CXWindowsScreen() { + assert(s_screen != NULL); assert(m_display == NULL); + + s_screen = NULL; } void CXWindowsScreen::openDisplay() { assert(m_display == NULL); + // set the X I/O error handler so we catch the display disconnecting + XSetIOErrorHandler(&CXWindowsScreen::ioErrorHandler); + // open the display log((CLOG_DEBUG "XOpenDisplay(%s)", "NULL")); m_display = XOpenDisplay(NULL); // FIXME -- allow non-default if (m_display == NULL) - throw int(5); // FIXME -- make exception for this + throw XScreenOpenFailure(); // get default screen m_screen = DefaultScreen(m_display); @@ -50,7 +61,7 @@ void CXWindowsScreen::openDisplay() m_root = RootWindow(m_display, m_screen); // let subclass prep display - onOpenDisplay(); + onOpenDisplay(m_display); // initialize clipboards for (ClipboardID id = 0; id < kClipboardEnd; ++id) { @@ -60,10 +71,10 @@ void CXWindowsScreen::openDisplay() void CXWindowsScreen::closeDisplay() { - assert(m_display != NULL); + CLock lock(&m_mutex); // let subclass close down display - onCloseDisplay(); + onCloseDisplay(m_display); // destroy clipboards for (ClipboardID id = 0; id < kClipboardEnd; ++id) { @@ -71,9 +82,12 @@ void CXWindowsScreen::closeDisplay() } // close the display - XCloseDisplay(m_display); - m_display = NULL; - log((CLOG_DEBUG "closed display")); + if (m_display != NULL) { + XCloseDisplay(m_display); + m_display = NULL; + log((CLOG_DEBUG "closed display")); + } + XSetIOErrorHandler(NULL); } int CXWindowsScreen::getScreen() const @@ -164,7 +178,7 @@ bool CXWindowsScreen::getEvent(XEvent* xevent) const void CXWindowsScreen::doStop() { - CLock lock(&m_mutex); + // caller must have locked display m_stop = true; } @@ -179,6 +193,11 @@ ClipboardID CXWindowsScreen::getClipboardID(Atom selection) const return kClipboardEnd; } +void CXWindowsScreen::onUnexpectedClose() +{ + // do nothing +} + bool CXWindowsScreen::processEvent(XEvent* xevent) { switch (xevent->type) { @@ -326,6 +345,21 @@ void CXWindowsScreen::destroyClipboardRequest( } } +int CXWindowsScreen::ioErrorHandler(Display*) +{ + // the display has disconnected, probably because X is shutting + // down. X forces us to exit at this point. that's arguably + // a flaw in X but, realistically, it's difficult to gracefully + // handle not having a Display* anymore. we'll simply log the + // error, notify the subclass (which must not use the display + // so we set it to NULL), and exit. + log((CLOG_WARN "X display has unexpectedly disconnected")); + s_screen->m_display = NULL; + s_screen->onUnexpectedClose(); + log((CLOG_CRIT "quiting due to X display disconnection")); + exit(1); +} + // // CXWindowsScreen::CDisplayLock diff --git a/synergy/CXWindowsScreen.h b/synergy/CXWindowsScreen.h index b2a675ed..a100d22a 100644 --- a/synergy/CXWindowsScreen.h +++ b/synergy/CXWindowsScreen.h @@ -52,7 +52,8 @@ protected: // wait for and get the next X event. cancellable. bool getEvent(XEvent*) const; - // cause getEvent() to return false immediately and forever after + // cause getEvent() to return false immediately and forever after. + // the caller must have locked the display. void doStop(); // set the contents of the clipboard (i.e. primary selection) @@ -63,15 +64,21 @@ protected: bool getDisplayClipboard(ClipboardID, IClipboard* clipboard) const; - // called by openDisplay() to allow subclasses to prepare the display - virtual void onOpenDisplay() = 0; + // called by openDisplay() to allow subclasses to prepare the display. + // the display is locked and passed to the subclass. + virtual void onOpenDisplay(Display*) = 0; // called by openDisplay() after onOpenDisplay() to create each clipboard virtual CXWindowsClipboard* createClipboard(ClipboardID) = 0; - // called by closeDisplay() to - virtual void onCloseDisplay() = 0; + // called by closeDisplay() to allow subclasses to clean up the display. + // the display is locked and passed to the subclass. note that the + // display may be NULL if the display has unexpectedly disconnected. + virtual void onCloseDisplay(Display*) = 0; + + // called if the display is unexpectedly closing. default does nothing. + virtual void onUnexpectedClose(); // called when a clipboard is lost virtual void onLostClipboard(ClipboardID) = 0; @@ -91,6 +98,9 @@ private: // terminate a selection request void destroyClipboardRequest(Window window); + // X I/O error handler + static int ioErrorHandler(Display*); + private: Display* m_display; int m_screen; @@ -103,6 +113,10 @@ private: // X is not thread safe CMutex m_mutex; + + // pointer to (singleton) screen. this is only needed by + // ioErrorHandler(). + static CXWindowsScreen* s_screen; }; #endif diff --git a/synergy/Makefile b/synergy/Makefile index 3d499dcd..c09c122d 100644 --- a/synergy/Makefile +++ b/synergy/Makefile @@ -24,6 +24,7 @@ CXXFILES = \ CXWindowsClipboard.cpp \ CXWindowsScreen.cpp \ CXWindowsUtil.cpp \ + XScreen.cpp \ XSynergy.cpp \ $(NULL) diff --git a/synergy/XScreen.cpp b/synergy/XScreen.cpp new file mode 100644 index 00000000..4466525f --- /dev/null +++ b/synergy/XScreen.cpp @@ -0,0 +1,10 @@ +#include "XScreen.h" + +// +// XScreenOpenFailure +// + +CString XScreenOpenFailure::getWhat() const throw() +{ + return "XScreenOpenFailure"; +} diff --git a/synergy/XScreen.h b/synergy/XScreen.h new file mode 100644 index 00000000..23ea45e5 --- /dev/null +++ b/synergy/XScreen.h @@ -0,0 +1,14 @@ +#ifndef XSCREEN_H +#define XSCREEN_H + +#include "XBase.h" + +class XScreen : public XBase { }; + +// screen cannot be opened +class XScreenOpenFailure : public XScreen { +protected: + virtual CString getWhat() const throw(); +}; + +#endif diff --git a/synergy/synergy.dsp b/synergy/synergy.dsp index a14a9587..cf76d74a 100644 --- a/synergy/synergy.dsp +++ b/synergy/synergy.dsp @@ -115,6 +115,10 @@ SOURCE=.\CTCPSocketFactory.cpp # End Source File # Begin Source File +SOURCE=.\XScreen.cpp +# End Source File +# Begin Source File + SOURCE=.\XSynergy.cpp # End Source File # End Group @@ -187,6 +191,10 @@ SOURCE=.\ProtocolTypes.h # End Source File # Begin Source File +SOURCE=.\XScreen.h +# End Source File +# Begin Source File + SOURCE=.\XSynergy.h # End Source File # End Group