From e80c2baea0319decdad80c3c98cc7b28a010b0f0 Mon Sep 17 00:00:00 2001 From: Timothy Pearson Date: Wed, 8 Apr 2015 15:13:08 -0500 Subject: Remove external dcop call and associated thread Fix lockup on lock screen command due to signal race condition --- kdesktop/desktop.cc | 10 +++-- kdesktop/desktop.h | 5 ++- kdesktop/krootwm.cc | 72 +++++---------------------------- kdesktop/krootwm.h | 27 ++----------- kdesktop/lock/lockprocess.cc | 4 +- kdesktop/lock/main.cc | 7 +++- kdesktop/lockeng.cc | 96 +++++++++++++++++++++++++++++++------------- kdesktop/lockeng.h | 18 +++++++-- kdesktop/main.cc | 2 +- 9 files changed, 115 insertions(+), 126 deletions(-) diff --git a/kdesktop/desktop.cc b/kdesktop/desktop.cc index de6873e2c..2732bff08 100644 --- a/kdesktop/desktop.cc +++ b/kdesktop/desktop.cc @@ -134,12 +134,14 @@ bool KRootWidget::eventFilter ( TQObject *, TQEvent * e ) KDesktop::WheelDirection KDesktop::m_eWheelDirection = KDesktop::m_eDefaultWheelDirection; const char* KDesktop::m_wheelDirectionStrings[2] = { "Forward", "Reverse" }; -KDesktop::KDesktop( bool x_root_hack, bool wait_for_kded ) : +KDesktop::KDesktop( SaverEngine* saver, bool x_root_hack, bool wait_for_kded ) : TQWidget( 0L, "desktop", (WFlags)(WResizeNoErase | ( x_root_hack ? (WStyle_Customize | WStyle_NoBorder) : 0)) ), KDesktopIface(), // those two WStyle_ break kdesktop when the root-hack isn't used (no Dnd) startup_id( NULL ), m_waitForKicker(0) { + m_pSaver = saver; + NETRootInfo i( tqt_xdisplay(), NET::Supported ); m_wmSupport = i.isSupported( NET::WM2ShowingDesktop ); @@ -249,7 +251,7 @@ KDesktop::initRoot() if (!m_bInit) { delete KRootWm::self(); - KRootWm* krootwm = new KRootWm( this ); // handler for root menu (used by kdesktop on RMB click) + KRootWm* krootwm = new KRootWm( m_pSaver, this ); // handler for root menu (used by kdesktop on RMB click) keys->setSlot("Lock Session", krootwm, TQT_SLOT(slotLock())); keys->updateConnections(); } @@ -327,7 +329,7 @@ KDesktop::initRoot() { m_pIconView->start(); delete KRootWm::self(); - KRootWm* krootwm = new KRootWm( this ); // handler for root menu (used by kdesktop on RMB click) + KRootWm* krootwm = new KRootWm( m_pSaver, this ); // handler for root menu (used by kdesktop on RMB click) keys->setSlot("Lock Session", krootwm, TQT_SLOT(slotLock())); keys->updateConnections(); } @@ -395,7 +397,7 @@ KDesktop::slotStart() // Global keys keys = new TDEGlobalAccel( TQT_TQOBJECT(this) ); - (void) new KRootWm( this ); + (void) new KRootWm( m_pSaver, this ); #include "kdesktopbindings.cpp" diff --git a/kdesktop/desktop.h b/kdesktop/desktop.h index 6d8015f31..c6a208f07 100644 --- a/kdesktop/desktop.h +++ b/kdesktop/desktop.h @@ -40,6 +40,7 @@ class StartupId; class KDIconView; class Minicli; class TDEActionCollection; +class SaverEngine; class KRootWidget : public TQObject { @@ -68,7 +69,7 @@ public: enum WheelDirection { Forward = 0, Reverse }; - KDesktop(bool x_root_hack, bool wait_for_kded ); + KDesktop(SaverEngine*, bool x_root_hack, bool wait_for_kded ); ~KDesktop(); // Implementation of the DCOP interface @@ -196,6 +197,8 @@ private: KDIconView *m_pIconView; KRootWidget *m_pRootWidget; + SaverEngine *m_pSaver; + Minicli *m_miniCli; StartupId* startup_id; diff --git a/kdesktop/krootwm.cc b/kdesktop/krootwm.cc index 756d0b5b5..d12e024ab 100644 --- a/kdesktop/krootwm.cc +++ b/kdesktop/krootwm.cc @@ -60,6 +60,7 @@ #include "desktop.h" #include "kcustommenu.h" #include "kdesktopsettings.h" +#include "lockeng.h" #include #include @@ -71,21 +72,11 @@ KRootWm * KRootWm::s_rootWm = 0; extern TQCString kdesktop_name, kicker_name, twin_name; -KRootWm::KRootWm(KDesktop* _desktop) : TQObject(_desktop), startup(FALSE) +KRootWm::KRootWm(SaverEngine* _saver, KDesktop* _desktop) : TQObject(_desktop), startup(FALSE) { - m_helperThread = new TQEventLoopThread; - m_helperThread->start(); - m_threadHelperObject = new KRootWmThreadHelperObject; - m_threadHelperObject->moveToThread(m_helperThread); - connect(this, TQT_SIGNAL(initializeHelperThread()), m_threadHelperObject, TQT_SLOT(initializeThread())); - connect(this, TQT_SIGNAL(terminateHelperThread()), m_threadHelperObject, TQT_SLOT(terminateThread())); - connect(this, TQT_SIGNAL(asyncLock()), m_threadHelperObject, TQT_SLOT(slotLock())); - connect(this, TQT_SIGNAL(asyncLockAndDoNewSession()), m_threadHelperObject, TQT_SLOT(lockAndDoNewSession())); - connect(this, TQT_SIGNAL(asyncSlotSessionActivated(int)), m_threadHelperObject, TQT_SLOT(slotSessionActivated(int))); - initializeHelperThread(); - s_rootWm = this; m_actionCollection = new TDEActionCollection(_desktop, this, "KRootWm::m_actionCollection"); + m_pSaver = _saver; m_pDesktop = _desktop; m_bDesktopEnabled = (m_pDesktop->iconView() != 0); customMenu1 = 0; @@ -226,11 +217,6 @@ KRootWm::KRootWm(KDesktop* _desktop) : TQObject(_desktop), startup(FALSE) KRootWm::~KRootWm() { - terminateHelperThread(); - m_helperThread->wait(); - delete m_threadHelperObject; - delete m_helperThread; - delete m_actionCollection; delete desktopMenu; delete windowListMenu; @@ -838,7 +824,8 @@ void KRootWm::slotCascadeWindows() { void KRootWm::slotLock() { - asyncLock(); + m_pSaver->lockScreen(); + m_pSaver->waitForLockEngage(); } @@ -882,49 +869,10 @@ void KRootWm::slotPopulateSessions() } } -void KRootWmThreadHelperObject::initializeThread() { - // Prevent kdesktop_lock signals from being handled by the wrong (non-GUI) thread - sigset_t set; - sigemptyset(&set); - sigaddset(&set, SIGUSR1); - sigaddset(&set, SIGUSR2); - sigaddset(&set, SIGTTIN); - pthread_sigmask(SIG_BLOCK, &set, NULL); -} - -void KRootWmThreadHelperObject::terminateThread() { - TQEventLoop* eventLoop = TQApplication::eventLoop(); - if (eventLoop) { - eventLoop->exit(0); - } -} - -void KRootWmThreadHelperObject::slotLock() { - // Block here until lock is complete - // If this is not done the desktop of the locked session will be shown after VT switch until the lock fully engages! - // Force remote call to ensure that blocking is enforced even though this call is being made from inside the kdesktop_name application... - // If this is not done DCOP will translate the call into a send and the desktop of the locked session will be shown after VT switch as above - system("dcop kdesktop KScreensaverIface lock"); -} - -void KRootWmThreadHelperObject::lockAndDoNewSession() { - // Block here until lock is complete - // If this is not done the desktop of the locked session will be shown after VT switch until the lock fully engages! - // Force remote call to ensure that blocking is enforced even though this call is being made from inside the kdesktop_name application... - // If this is not done DCOP will translate the call into a send and the desktop of the locked session will be shown after VT switch as above - if (system("dcop kdesktop KScreensaverIface lock") == 0) { - DM().startReserve(); - } -} - -void KRootWmThreadHelperObject::slotSessionActivated(int vt) { - DM().lockSwitchVT( vt ); -} - void KRootWm::slotSessionActivated( int ent ) { if (ent > 0 && !sessionsMenu->isItemChecked( ent )) { - asyncSlotSessionActivated( ent ); + DM().lockSwitchVT( ent ); } } @@ -962,11 +910,11 @@ void KRootWm::doNewSession( bool lock ) return; if (lock) { - asyncLockAndDoNewSession(); - } - else { - DM().startReserve(); + m_pSaver->lockScreen(); + m_pSaver->waitForLockEngage(); } + + DM().startReserve(); } void KRootWm::slotMenuItemActivated(int /* item */ ) diff --git a/kdesktop/krootwm.h b/kdesktop/krootwm.h index 0e27001b2..8fb5af8b9 100644 --- a/kdesktop/krootwm.h +++ b/kdesktop/krootwm.h @@ -33,6 +33,7 @@ typedef XID Window; class KMenuBar; class KDesktop; +class SaverEngine; class TQPopupMenu; class KCMultiDialog; class KNewMenu; @@ -66,7 +67,7 @@ class KRootWm: public TQObject { Q_OBJECT public: - KRootWm(KDesktop*); + KRootWm(SaverEngine*, KDesktop*); ~KRootWm(); bool startup; @@ -126,14 +127,8 @@ public slots: void slotOpenTerminal(); void slotLockNNewSession(); -signals: - void initializeHelperThread(); - void terminateHelperThread(); - void asyncLock(); - void asyncLockAndDoNewSession(); - void asyncSlotSessionActivated(int vt); - private: + SaverEngine* m_pSaver; KDesktop* m_pDesktop; // The five root menus : @@ -176,10 +171,6 @@ private: static KRootWm * s_rootWm; - TQEventLoopThread* m_helperThread; - KRootWmThreadHelperObject* m_threadHelperObject; - - private slots: void slotMenuItemActivated(int); @@ -188,16 +179,4 @@ private slots: void slotConfigClosed(); }; -class KRootWmThreadHelperObject : public TQObject -{ - TQ_OBJECT - - public slots: - void initializeThread(); - void terminateThread(); - void slotLock(); - void lockAndDoNewSession(); - void slotSessionActivated(int vt); -}; - #endif diff --git a/kdesktop/lock/lockprocess.cc b/kdesktop/lock/lockprocess.cc index 9612f5cca..4e3922a68 100644 --- a/kdesktop/lock/lockprocess.cc +++ b/kdesktop/lock/lockprocess.cc @@ -981,8 +981,6 @@ void LockProcess::createSaverWindow() } } - fullyOnline(); - kdDebug(1204) << "Saver window Id: " << winId() << endl; } @@ -1319,6 +1317,7 @@ void LockProcess::saverReadyIfNeeded() // Make sure the desktop is hidden before notifying the desktop that the saver is running m_notifyReadyRequested = false; saverReady(); + fullyOnline(); } } @@ -1405,6 +1404,7 @@ bool LockProcess::startSaver(bool notify_ready) } } } + return true; } diff --git a/kdesktop/lock/main.cc b/kdesktop/lock/main.cc index 79799d129..bafa02539 100644 --- a/kdesktop/lock/main.cc +++ b/kdesktop/lock/main.cc @@ -380,6 +380,7 @@ int main( int argc, char **argv ) kdesktop_pid = atoi(args->getOption( "internal" )); while (signalled_run == FALSE) { sigset_t new_mask; + sigset_t orig_mask; struct sigaction act; in_internal_mode = TRUE; @@ -438,7 +439,11 @@ int main( int argc, char **argv ) app->processEvents(); // wait for SIGUSR1, SIGUSR2, SIGWINCH, SIGTTIN, or SIGTTOU - sigsuspend(&new_mask); + sigprocmask(SIG_BLOCK, &new_mask, &orig_mask); + if (signalled_run != TRUE) { + sigsuspend(&orig_mask); + } + sigprocmask(SIG_UNBLOCK, &new_mask, NULL); // Reenable reception of X11 events on the root window XSelectInput( tqt_xdisplay(), tqt_xrootwin(), rootAttr.your_event_mask ); diff --git a/kdesktop/lockeng.cc b/kdesktop/lockeng.cc index 6d7131253..a5f3c313d 100644 --- a/kdesktop/lockeng.cc +++ b/kdesktop/lockeng.cc @@ -82,31 +82,29 @@ SaverEngine::SaverEngine() dBusWatch(0), systemdSession(0) { - struct sigaction act; - // handle SIGUSR1 m_masterSaverEngine = this; - act.sa_handler= sigusr1_handler; - sigemptyset(&(act.sa_mask)); - sigaddset(&(act.sa_mask), SIGUSR1); - act.sa_flags = 0; - sigaction(SIGUSR1, &act, 0L); + mSignalAction.sa_handler= sigusr1_handler; + sigemptyset(&(mSignalAction.sa_mask)); + sigaddset(&(mSignalAction.sa_mask), SIGUSR1); + mSignalAction.sa_flags = 0; + sigaction(SIGUSR1, &mSignalAction, 0L); // handle SIGUSR2 m_masterSaverEngine = this; - act.sa_handler= sigusr2_handler; - sigemptyset(&(act.sa_mask)); - sigaddset(&(act.sa_mask), SIGUSR2); - act.sa_flags = 0; - sigaction(SIGUSR2, &act, 0L); + mSignalAction.sa_handler= sigusr2_handler; + sigemptyset(&(mSignalAction.sa_mask)); + sigaddset(&(mSignalAction.sa_mask), SIGUSR2); + mSignalAction.sa_flags = 0; + sigaction(SIGUSR2, &mSignalAction, 0L); // handle SIGTTIN m_masterSaverEngine = this; - act.sa_handler= sigttin_handler; - sigemptyset(&(act.sa_mask)); - sigaddset(&(act.sa_mask), SIGTTIN); - act.sa_flags = 0; - sigaction(SIGTTIN, &act, 0L); + mSignalAction.sa_handler= sigttin_handler; + sigemptyset(&(mSignalAction.sa_mask)); + sigaddset(&(mSignalAction.sa_mask), SIGTTIN); + mSignalAction.sa_flags = 0; + sigaction(SIGTTIN, &mSignalAction, 0L); // Save X screensaver parameters XGetScreenSaver(tqt_xdisplay(), &mXTimeout, &mXInterval, @@ -164,22 +162,35 @@ SaverEngine::~SaverEngine() } //--------------------------------------------------------------------------- - +// // This should be called only using DCOP. +// void SaverEngine::lock() +{ + lockScreen(true); +} + +//--------------------------------------------------------------------------- +// +// Lock the screen +// +void SaverEngine::lockScreen(bool DCOP) { bool ok = true; if (mState != Saving) { - mSAKProcess->kill(SIGTERM); ok = startLockProcess( ForceLock ); // It takes a while for kdesktop_lock to start and lock the screen. // Therefore delay the DCOP call until it tells kdesktop that the locking is in effect. // This is done only for --forcelock . if( ok && mState != Saving ) { - DCOPClientTransaction* trans = kapp->dcopClient()->beginTransaction(); - mLockTransactions.append( trans ); + if (DCOP) { + DCOPClientTransaction* trans = kapp->dcopClient()->beginTransaction(); + if (trans) { + mLockTransactions.append( trans ); + } + } } } else @@ -203,7 +214,7 @@ void SaverEngine::processLockTransactions() void SaverEngine::saverLockReady() { - if( mState != Preparing ) + if( mState != Engaging ) { kdDebug( 1204 ) << "Got unexpected saverReady()" << endl; } @@ -216,7 +227,6 @@ void SaverEngine::save() { if (mState == Waiting) { - mSAKProcess->kill(SIGTERM); startLockProcess( DefaultLock ); } } @@ -224,7 +234,7 @@ void SaverEngine::save() //--------------------------------------------------------------------------- void SaverEngine::quit() { - if (mState == Saving || mState == Preparing) + if (mState == Saving || mState == Engaging) { stopLockProcess(); } @@ -324,6 +334,10 @@ void SaverEngine::slotSAKProcessExited() printf("[kdesktop] SAK driven secure dialog is not available for use (retcode %d). Check tdmtsak for proper functionality.\n", retcode); fflush(stdout); } + if (mState == Preparing) { + return; + } + if ((mSAKProcess->normalExit()) && (trinity_lockeng_sak_available == TRUE)) { bool ok = true; if (mState == Waiting) @@ -419,16 +433,22 @@ bool SaverEngine::restartDesktopLockProcess() // bool SaverEngine::startLockProcess( LockType lock_type ) { + int ret; + if (mState == Saving) { return true; } + mState = Preparing; + mSAKProcess->kill(SIGTERM); + enableExports(); kdDebug(1204) << "SaverEngine: starting saver" << endl; emitDCOPSignal("KDE_start_screensaver()", TQByteArray()); if (!restartDesktopLockProcess()) { + mState = Waiting; return false; } @@ -450,10 +470,14 @@ bool SaverEngine::startLockProcess( LockType lock_type ) mLockProcess.kill(SIGTTIN); // Request blanking } - mLockProcess.kill(SIGTTOU); // Start lock + ret = mLockProcess.kill(SIGTTOU); // Start lock + if (!ret) { + mState = Waiting; + return false; + } XSetScreenSaver(tqt_xdisplay(), 0, mXInterval, PreferBlanking, mXExposures); - mState = Preparing; + mState = Engaging; if (mXAutoLock) { mXAutoLock->stop(); @@ -601,7 +625,6 @@ void SaverEngine::idleTimeout() // disable X screensaver XForceScreenSaver(tqt_xdisplay(), ScreenSaverReset ); XSetScreenSaver(tqt_xdisplay(), 0, mXInterval, PreferBlanking, DontAllowExposures); - mSAKProcess->kill(SIGTERM); startLockProcess( DefaultLock ); } @@ -771,7 +794,7 @@ void SaverEngine::handleDBusSignal(const TQT_DBusMessage& msg) { && msg.path() == systemdSession->path() && msg.interface() == SYSTEMD_LOGIN1_SESSION_IFACE && msg.member() == "Lock") { - lock(); + lockScreen(); return; } @@ -784,3 +807,20 @@ void SaverEngine::handleDBusSignal(const TQT_DBusMessage& msg) { return; } } + +void SaverEngine::waitForLockEngage() { + sigset_t new_mask; + sigset_t orig_mask; + + // wait for SIGUSR1, SIGUSR2, SIGTTIN + sigemptyset(&new_mask); + sigaddset(&new_mask, SIGUSR1); + sigaddset(&new_mask, SIGUSR2); + sigaddset(&new_mask, SIGTTIN); + + sigprocmask(SIG_BLOCK, &new_mask, &orig_mask); + while ((mState != Waiting) && (mState != Saving)) { + sigsuspend(&orig_mask); + } + sigprocmask(SIG_UNBLOCK, &new_mask, NULL); +} \ No newline at end of file diff --git a/kdesktop/lockeng.h b/kdesktop/lockeng.h index 70bde3281..8da2c7959 100644 --- a/kdesktop/lockeng.h +++ b/kdesktop/lockeng.h @@ -82,6 +82,17 @@ public: */ virtual void saverLockReady(); + /** + * @internal + */ + void lockScreen(bool DCOP = false); + + /** + * Called by KDesktop to wait for saver engage + * @internal + */ + void waitForLockEngage(); + public slots: void slotLockProcessWaiting(); void slotLockProcessFullyActivated(); @@ -113,6 +124,7 @@ private: void onDBusServiceUnregistered(const TQString&); protected: + enum SaverState { Waiting, Preparing, Engaging, Saving }; enum LockType { DontLock, DefaultLock, ForceLock, SecureDialog }; bool startLockProcess( LockType lock_type ); void stopLockProcess(); @@ -121,12 +133,11 @@ protected: xautolock_corner_t applyManualSettings(int); protected: - enum State { Waiting, Preparing, Saving }; bool mEnabled; - State mState; + SaverState mState; XAutoLock *mXAutoLock; - TDEProcess mLockProcess; + TDEProcess mLockProcess; int mTimeout; // the original X screensaver parameters @@ -142,6 +153,7 @@ private: TDEProcess* mSAKProcess; bool mTerminationRequested; bool mSaverProcessReady; + struct sigaction mSignalAction; TQT_DBusConnection dBusConn; TQT_DBusProxy* dBusLocal; TQT_DBusProxy* dBusWatch; diff --git a/kdesktop/main.cc b/kdesktop/main.cc index 4228df0c8..b54c03cb7 100644 --- a/kdesktop/main.cc +++ b/kdesktop/main.cc @@ -283,7 +283,7 @@ extern "C" KDE_EXPORT int kdemain( int argc, char **argv ) TDESelectionOwner kde_running( "_KDE_RUNNING", 0 ); kde_running.claim( false ); - KDesktop desktop( x_root_hack, wait_for_kded ); + KDesktop desktop( &saver, x_root_hack, wait_for_kded ); args->clear(); -- cgit v1.2.3