kdesktop: make sure the saver/lock remains responsive even when invokation has failed.

This could happen for example when a popup menu is open and the saver
autolock timeout expires. Before this commit, both kdesktop and the
X11 server would be in a dirty start and the saver/lock would either not
work anymore or not hide the windows on the screen.
It was necessary to kill and restart kdesktop to restore proper
functionality.

Note: the conditional return value of 0 or 12 at main.cpp:554 is a temporary
measure to prevent the screen to forcibly lock if the user forgets a popup
menu open and the saver timeout expires. Once the issue with input grabbing
is solved, we can remove the conditional return value and simply return an error
in such condition. Same applies to the variable trinity_desktop_lock_failed_grab,
which will no longer be needed in such case.o

Signed-off-by: Michele Calgaro <michele.calgaro@yahoo.it>

3 files changed, 83 insertions, 42 deletions

diff --git a/kdesktop/lock/lockprocess.cpp b/kdesktop/lock/lockprocess.cpp
index 789ceee0f..48af0496e 100644
--- a/kdesktop/lock/lockprocess.cpp
+++ b/kdesktop/lock/lockprocess.cpp
@@ -151,14 +151,6 @@ Atom kde_wm_system_modal_notification = 0;
 Atom kde_wm_transparent_to_desktop = 0;
 Atom kde_wm_transparent_to_black = 0;
 
-static void segv_handler(int)
-{
-	kdError(KDESKTOP_DEBUG_ID) << "A fatal exception was encountered."
-		<< " Trapping and ignoring it so as not to compromise desktop security..."
-		<< kdBacktrace() << endl;
-	sleep(1);
-}
-
 extern Atom tqt_wm_state;
 extern bool trinity_desktop_lock_use_system_modal_dialogs;
 extern bool trinity_desktop_lock_delay_screensaver_start;
@@ -166,6 +158,7 @@ extern bool trinity_desktop_lock_use_sak;
 extern bool trinity_desktop_lock_hide_active_windows;
 extern bool trinity_desktop_lock_hide_cancel_button;
 extern bool trinity_desktop_lock_forced;
+extern bool trinity_desktop_lock_failed_grab;
 
 extern LockProcess* trinity_desktop_lock_process;
 
@@ -176,6 +169,21 @@ extern TQXLibWindowList trinity_desktop_lock_hidden_window_list;
 
 bool trinity_desktop_lock_autohide_lockdlg = true;
 
+static void segv_handler(int)
+{
+	// Try to send a USR1 signal to kdesktop to make sure it does not get
+	// stuck into an `Engaging` state in case kdesktop_lock crashes.
+	// This prevents the locking mechanism from becaming unresponsive
+	// in case of exceptions.
+	kill(kdesktop_pid, SIGUSR1);
+
+	kdError(KDESKTOP_DEBUG_ID) << "A fatal exception was encountered."
+		<< " Trapping and ignoring it so as not to compromise desktop security..."
+		<< kdBacktrace() << endl;
+
+	sleep(1);
+}
+
 #define ENABLE_CONTINUOUS_LOCKDLG_DISPLAY \
 if (!mForceContinualLockDisplayTimer->isActive()) mForceContinualLockDisplayTimer->start(100, false); \
 trinity_desktop_lock_autohide_lockdlg = false; \
@@ -234,7 +242,9 @@ LockProcess::LockProcess()
 	m_notifyReadyRequested(false),
 	m_loginCardDevice(NULL),
 	m_maskWidget(NULL),
-	m_saverRootWindow(0)
+	m_saverRootWindow(0),
+	mControlPipeHandler(nullptr),
+	mControlPipeHandlerThread(nullptr)
 {
 #ifdef KEEP_MOUSE_UNGRABBED
 	setNFlags(WX11DisableMove|WX11DisableClose|WX11DisableShade|WX11DisableMinimize|WX11DisableMaximize);
@@ -331,8 +341,6 @@ LockProcess::~LockProcess()
 	mControlPipeHandler->terminateThread();
 	mControlPipeHandlerThread->wait();
 	delete mControlPipeHandler;
-// 	delete mControlPipeHandlerThread;
-
 	if (resizeTimer != NULL) {
 		resizeTimer->stop();
 		delete resizeTimer;
@@ -412,9 +420,7 @@ void LockProcess::init(bool child, bool useBlankOnly)
 	}
 #endif
 
-#if (TQT_VERSION-0 >= 0x030200) // XRANDR support
 	connect( tdeApp->desktop(), TQ_SIGNAL( resized( int )), TQ_SLOT( desktopResized()));
-#endif
 
 	if (!trinity_desktop_lock_use_system_modal_dialogs) {
 		setWFlags((WFlags)WX11BypassWM);
@@ -438,6 +444,12 @@ void LockProcess::init(bool child, bool useBlankOnly)
 	TQObject::connect(mControlPipeHandler, TQ_SIGNAL(processCommand(TQString)), this, TQ_SLOT(processInputPipeCommand(TQString)));
 	TQTimer::singleShot(0, mControlPipeHandler, TQ_SLOT(run()));
 	mControlPipeHandlerThread->start();
+	// If the lock process terminates before 'mControlPipeHandler::run()' has been called, the
+	// 'mControlPipeHandlerThread' thread would not terminate and the lock process would have a
+	// dirty exit, potentially leaving 'kdesktop' in a dirty state that prevents the lock from
+	// working correctly till 'kdesktop' is killed and restarted. By forcing a call to 'processEvents()'
+	// we make sure to handle pending timer events and execute the required call
+	tdeApp->processEvents();
 }
 
 static int signal_pipe[2];
@@ -566,7 +578,7 @@ bool LockProcess::lock()
 	m_startupStatusDialog->setStatusMessage(i18n("Securing desktop session").append("..."));
 	m_startupStatusDialog->show();
 	m_startupStatusDialog->setActiveWindow();
-	tqApp->processEvents();
+	tdeApp->processEvents();
 #endif
 
 	if (startSaver(true)) {
@@ -722,7 +734,7 @@ bool LockProcess::runSecureDialog()
 	m_startupStatusDialog->setStatusMessage(i18n("Securing desktop session").append("..."));
 	m_startupStatusDialog->show();
 	m_startupStatusDialog->setActiveWindow();
-	tqApp->processEvents();
+	tdeApp->processEvents();
 #endif
 
 	mInSecureDialog = true;
@@ -1343,6 +1355,7 @@ bool LockProcess::startSaver(bool notify_ready)
 	if (!child_saver && !grabInput())
 	{
 		kdWarning(KDESKTOP_DEBUG_ID) << "LockProcess::startSaver() grabInput() failed!!!!" << endl;
+		trinity_desktop_lock_failed_grab = true;
 		return false;
 	}
 	mBusy = false;
@@ -2308,7 +2321,7 @@ bool LockProcess::x11Event(XEvent *event)
 		&& event->xkey.window != mDialogs.first()->winId()) {
 		XEvent ev2 = *event;
 		ev2.xkey.window = ev2.xkey.subwindow = mDialogs.first()->winId();
-		tqApp->x11ProcessEvent( &ev2 );
+		tdeApp->x11ProcessEvent( &ev2 );
 		return true;
 	}
 
@@ -2963,7 +2976,7 @@ void LockProcess::saverReady() {
 // Control pipe handler
 //
 ControlPipeHandlerObject::ControlPipeHandlerObject() : TQObject() {
-	mParent = NULL;
+	mParent = nullptr;
 	mRunning = false;
 	mTerminate = false;
 	mThreadID = 0L;
@@ -2987,7 +3000,7 @@ void ControlPipeHandlerObject::run(void) {
 	int display_number = atoi(TQString(XDisplayString(tqt_xdisplay())).replace(":","").ascii());
 
 	if (display_number < 0) {
-		printf("[kdesktop_lock] Warning: unable to create control socket.  Interactive logon modules may not function properly.\n");
+		printf("[kdesktop_lock] Warning: unable to create control socket. Interactive logon modules may not function properly.\n");
 		mRunning = false;
 		TQApplication::eventLoop()->exit(-1);
 		return;
@@ -3018,7 +3031,7 @@ void ControlPipeHandlerObject::run(void) {
 	}
 
 	if (!mParent->mPipeOpen) {
-		printf("[kdesktop_lock] Warning: unable to create control socket '%s'.  Interactive logon modules may not function properly.\n", fifo_file);
+		printf("[kdesktop_lock] Warning: unable to create control socket '%s'. Interactive logon modules may not function properly.\n", fifo_file);
 		mRunning = false;
 		TQApplication::eventLoop()->exit(-1);
 		return;
diff --git a/kdesktop/lock/main.cpp b/kdesktop/lock/main.cpp
index 41b918664..090d4ae6e 100644
--- a/kdesktop/lock/main.cpp
+++ b/kdesktop/lock/main.cpp
@@ -64,8 +64,10 @@ bool trinity_desktop_lock_use_sak = false;
 bool trinity_desktop_lock_hide_active_windows = false;
 bool trinity_desktop_lock_hide_cancel_button = false;
 bool trinity_desktop_lock_forced = false;
+// This is a temporary variable used till a fix for the grab issue is prepared
+bool trinity_desktop_lock_failed_grab = false;
 
-LockProcess* trinity_desktop_lock_process = NULL;
+LockProcess* trinity_desktop_lock_process = nullptr;
 
 bool signalled_forcelock;
 bool signalled_dontlock;
@@ -88,17 +90,17 @@ static void sigusr2_handler(int)
 	signalled_dontlock = true;
 }
 
-static void sigusr3_handler(int)
+static void sigwinch_handler(int)
 {
 	signalled_securedialog = true;
 }
 
-static void sigusr4_handler(int)
+static void sigttin_handler(int)
 {
 	signalled_blank = true;
 }
 
-static void sigusr5_handler(int)
+static void sigttou_handler(int)
 {
 	signalled_run = true;
 }
@@ -220,6 +222,19 @@ void restore_hidden_override_redirect_windows() {
 	}
 }
 
+static void shutdown_lock_process(bool unclean)
+{
+	if (unclean)
+	{
+		// Send a USR1 signal to kdesktop to make sure it does not get stuck into
+		// an `Engaging` state in case kdesktop_lock activation failed. This prevents
+		// the locking mechanism from becaming unresponsive in case of exceptions.
+		kill(kdesktop_pid, SIGUSR1);
+	}
+	delete trinity_desktop_lock_process;
+	trinity_desktop_lock_process = nullptr;
+}
+
 // -----------------------------------------------------------------------------
 
 int main( int argc, char **argv )
@@ -236,9 +251,9 @@ int main( int argc, char **argv )
 
 	XSetErrorHandler(trapXErrors);
 
-	MyApp* app = NULL;
+	MyApp *app = nullptr;
 
-	while (1 == 1) {
+	while (true) {
 		sigset_t new_mask;
 		sigset_t orig_mask;
 
@@ -261,7 +276,7 @@ int main( int argc, char **argv )
 
 		if (TDEGlobalSettings::isMultiHead()) {
 			Display *dpy = XOpenDisplay(NULL);
-			if (! dpy) {
+			if (!dpy) {
 				fprintf(stderr,
 					"%s: FATAL ERROR: couldn't open display '%s'\n",
 					argv[0], XDisplayName(NULL));
@@ -303,14 +318,11 @@ int main( int argc, char **argv )
 					}
 				}
 
-				env.sprintf("DISPLAY=%s.%d", display_name.data(),
-					kdesktop_screen_number);
+				env.sprintf("DISPLAY=%s.%d", display_name.data(), kdesktop_screen_number);
 				kdDebug() << "env " << env << endl;
 
 				if (putenv(strdup(env.data()))) {
-					fprintf(stderr,
-						"%s: WARNING: unable to set DISPLAY environment variable\n",
-						argv[0]);
+					fprintf(stderr, "%s: WARNING: unable to set DISPLAY environment variable\n", argv[0]);
 					perror("putenv()");
 				}
 			}
@@ -410,20 +422,20 @@ int main( int argc, char **argv )
 			sigaddset(&(act.sa_mask), SIGUSR2);
 			act.sa_flags = 0;
 			sigaction(SIGUSR2, &act, 0L);
-			// handle SIGWINCH (an ersatz SIGUSR3)
-			act.sa_handler= sigusr3_handler;
+			// handle SIGWINCH (as custom user signal rather than its inherent meaning)
+			act.sa_handler= sigwinch_handler;
 			sigemptyset(&(act.sa_mask));
 			sigaddset(&(act.sa_mask), SIGWINCH);
 			act.sa_flags = 0;
 			sigaction(SIGWINCH, &act, 0L);
-			// handle SIGTTIN (an ersatz SIGUSR4)
-			act.sa_handler= sigusr4_handler;
+			// handle SIGTTIN (as custom user signal rather than its inherent meaning)
+			act.sa_handler= sigttin_handler;
 			sigemptyset(&(act.sa_mask));
 			sigaddset(&(act.sa_mask), SIGTTIN);
 			act.sa_flags = 0;
 			sigaction(SIGTTIN, &act, 0L);
-			// handle SIGTTOU (an ersatz SIGUSR5)
-			act.sa_handler= sigusr5_handler;
+			// handle SIGTTOU (as custom user signal rather than its inherent meaning)
+			act.sa_handler= sigttou_handler;
 			sigemptyset(&(act.sa_mask));
 			sigaddset(&(act.sa_mask), SIGTTOU);
 			act.sa_flags = 0;
@@ -506,8 +518,9 @@ int main( int argc, char **argv )
 			trinity_desktop_lock_process->setParent(parent_connection);
 		}
 
+		trinity_desktop_lock_failed_grab = false;
 		bool rt;
-		if( (((!child) && (args->isSet( "forcelock" ))) || signalled_forcelock)) {
+		if( (!child && args->isSet( "forcelock" )) || signalled_forcelock) {
 			rt = trinity_desktop_lock_process->lock();
 		}
 		else if( child || (args->isSet( "dontlock" ) || signalled_dontlock)) {
@@ -519,25 +532,40 @@ int main( int argc, char **argv )
 				rt = trinity_desktop_lock_process->runSecureDialog();
 			}
 			else {
+				shutdown_lock_process(true);
 				return 1;
 			}
 		}
 		else {
 			rt = trinity_desktop_lock_process->defaultSave();
 		}
+
+		// Make sure to handle all pending responses from the X server.
+		// If we don't do this, in case of failed activation of the saver/lock screen,
+		// we will end up in a dirty state and the screen lock will no longer hide the windows
+		// on the screen due to 'm_rootPixmap' failing to load the background image.
+		// This is caused by a 'XConvertSelection' request in 'TDESharedPixmap::loadFromShared'
+		// not being handled and causing the corresponding property to become unusuable in X for
+		// subsequent lock requests.
+		XSync(tqt_xdisplay(), False);
+		app->processEvents();
+
 		if (!rt) {
-			return 0;
+			shutdown_lock_process(true);
+			return (trinity_desktop_lock_failed_grab ? 0 : 12);
 		}
 
 		if (!in_internal_mode) {
 			trinity_desktop_lock_hidden_window_list.clear();
 			int ret = app->exec();
 			restore_hidden_override_redirect_windows();
+			shutdown_lock_process(false);
 			return ret;
 		}
 		else {
 			if (kill(kdesktop_pid, 0) < 0) {
 				// The controlling kdesktop process probably died.  Commit suicide...
+				shutdown_lock_process(true);
 				return 12;
 			}
 			trinity_desktop_lock_hidden_window_list.clear();
@@ -545,11 +573,11 @@ int main( int argc, char **argv )
 			restore_hidden_override_redirect_windows();
 			if (kill(kdesktop_pid, SIGUSR1) < 0) {
 				// The controlling kdesktop process probably died.  Commit suicide...
+				shutdown_lock_process(true);
 				return 12;
 			}
 
-			delete trinity_desktop_lock_process;
-			trinity_desktop_lock_process = NULL;
+			shutdown_lock_process(false);
 
 			// FIXME
 			// We should not have to return (restart) at all,
diff --git a/kdesktop/lockeng.cpp b/kdesktop/lockeng.cpp
index 1392c9db3..57a42c9b5 100644
--- a/kdesktop/lockeng.cpp
+++ b/kdesktop/lockeng.cpp
@@ -108,7 +108,7 @@ SaverEngine::SaverEngine()
 	mSignalAction.sa_flags = 0;
 	sigaction(SIGUSR2, &mSignalAction, 0L);
 
-	// handle SIGTTIN
+	// handle SIGTTIN (as custom user signal rather than its inherent meaning)
 	mSignalAction.sa_handler= sigttin_handler;
 	sigemptyset(&(mSignalAction.sa_mask));
 	sigaddset(&(mSignalAction.sa_mask), SIGTTIN);