Remove external dcop call and associated thread

Fix lockup on lock screen command due to signal race condition

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 <netwm.h>
 #include <X11/X.h>
@@ -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();