add copywrite

Fix 5sec stall when switching from an application to be stopped to an application that was stopped before.
Make Xinstance thread safe

CMakeLists.txt

@@ -2,8 +2,8 @@ cmake_minimum_required(VERSION 2.4)
 
 project(sigstoped)
 
-set(SRC_FILES main.cpp process.cpp xinstance.cpp)
+set(SRC_FILES main.cpp process.cpp xinstance.cpp CppTimer.cpp)
-set(LIBS -lX11)
+set(LIBS -lX11 -lrt)
 
 add_executable(${PROJECT_NAME} ${SRC_FILES})
 

CppTimer.cpp

@@ -0,0 +1,97 @@
+#include "CppTimer.h"
+#include <fcntl.h>
+
+/**
+ * GNU GENERAL PUBLIC LICENSE
+ * Version 3, 29 June 2007
+ *
+ * (C) 2020, Bernd Porr <mail@bernporr.me.uk>
+ * 
+ * This is inspired by the timer_create man page.
+ **/
+
+CppTimer::CppTimer() {
+	// We create a static handler catches the signal SIG
+	sa.sa_flags = SA_SIGINFO;
+	sa.sa_sigaction = handler;
+	sigemptyset(&sa.sa_mask);
+	if (sigaction(SIG, &sa, NULL) == -1)
+		throw("Could not create signal handler");
+	
+	if(pipe(pipeFd) < 0)
+		throw("Could not create pipe");
+	
+	// Create the timer
+	sev.sigev_notify = SIGEV_SIGNAL;
+	sev.sigev_signo = SIG;
+	// Cruical is that the signal carries the pointer to this class instance here
+	// because the handler just handles anything that comes in!
+	sev.sigev_value.sival_ptr = this;
+	// create the timer
+	if (timer_create(CLOCKID, &sev, &timerid) == -1)
+		throw("Could not create timer");
+};
+
+void CppTimer::start(long secs, long nanosecs,std::function<void()> callbackIn, int type) {
+	switch(type){
+		case(PERIODIC):
+			//starts after specified period of nanoseconds
+			its.it_value.tv_sec = secs;
+			its.it_value.tv_nsec = nanosecs;
+			its.it_interval.tv_sec = secs;
+			its.it_interval.tv_nsec = nanosecs;
+			break;
+		case(ONESHOT):
+			//fires once after specified period of nanoseconds
+			its.it_value.tv_sec = secs;
+			its.it_value.tv_nsec = nanosecs;
+			its.it_interval.tv_sec = 0;
+			its.it_interval.tv_nsec = 0;
+			break;
+	}
+	callback = callbackIn;
+	if (timer_settime(timerid, 0, &its, NULL) == -1)
+		throw("Could not start timer");
+	discardPipe();
+	running = true;
+}
+
+void CppTimer::discardPipe() {
+	char buf;
+	fcntl(pipeFd[0], F_SETFL, O_NONBLOCK);
+	while(read(pipeFd[0], &buf, 1) > 0);
+	fcntl(pipeFd[0], F_SETFL, 0);
+}
+
+void CppTimer::block()
+{
+	if(running)
+	{
+		char buf;
+		read(pipeFd[0], &buf, 1);
+	}
+}
+
+void CppTimer::stop() {
+	// disarm
+	struct itimerspec itsnew;
+	itsnew.it_value.tv_sec = 0;
+	itsnew.it_value.tv_nsec = 0;
+	itsnew.it_interval.tv_sec = 0;
+	itsnew.it_interval.tv_nsec = 0;
+	timer_settime(timerid, 0, &itsnew, &its);
+	running = false;
+}
+
+bool CppTimer::isRunning()
+{
+	return running;
+}
+
+CppTimer::~CppTimer() {
+	stop();
+	// delete the timer
+	timer_delete(timerid);
+	// default action for signal handling
+	signal(SIG, SIG_IGN);
+}

CppTimer.h

@@ -0,0 +1,85 @@
+#ifndef __CPP_TIMER_H_
+#define __CPP_TIMER_H_
+
+/**
+ * GNU GENERAL PUBLIC LICENSE
+ * Version 3, 29 June 2007
+ *
+ * (C) 2020, Bernd Porr <mail@bernporr.me.uk>
+ * 
+ * This is inspired by the timer_create man page.
+ **/
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <signal.h>
+#include <time.h>
+#include <functional>
+#include <atomic>
+
+#define CLOCKID CLOCK_MONOTONIC
+#define SIG SIGRTMIN
+
+/**
+ * Timer class which repeatedly fires. It's wrapper around the
+ * POSIX per-process timer.
+ **/
+class CppTimer {
+
+public:
+	static constexpr int PERIODIC = 0;
+	static constexpr int ONESHOT  = 1;
+	static constexpr long MS_TO_NS  = 1000000;
+	/**
+	 * Creates an instance of the timer and connects the
+	 * signal handler to the timer.
+	 **/
+	CppTimer();
+
+	/**
+	 * Starts the timer. The timer fires first after
+	 * the specified time in nanoseconds and then at
+	 * that interval in PERIODIC mode. In ONESHOT mode
+	 * the timer fires once after the specified time in
+	 * nanoseconds.
+	 **/
+	virtual void start(long secs, long nanosecs, std::function<void()> callbackIn, int type = PERIODIC); 
+
+	/**
+	* Stops the timer by disarming it. It can be re-started
+	* with start().
+	**/
+	virtual void stop();
+
+	/**
+	 * Destructor disarms the timer, deletes it and
+	 * disconnect the signal handler.
+	 **/
+	virtual ~CppTimer();
+	
+	bool isRunning();
+	
+	void block();
+
+private:
+	timer_t timerid = 0;
+	struct sigevent sev;
+	struct sigaction sa;
+	struct itimerspec its;
+	int pipeFd[2];
+	std::atomic<bool> running = false;
+	std::function<void()> callback;
+	void discardPipe();
+		
+	static void handler(int sig, siginfo_t *si, void *uc ) {
+		CppTimer *timer = reinterpret_cast<CppTimer *> (si->si_value.sival_ptr);
+		timer->callback();
+		char buf = '\n'; 
+		write(timer->pipeFd[1], &buf, 1);
+		timer->running = false;
+	}
+};
+
+
+#endif

argpopt.h

@@ -23,9 +23,10 @@
 struct Config
 {
     bool ignoreClientMachine = false;
+    int  timeoutSecs = 10;
 };
 
-const char *argp_program_version = "1.0.2";
+const char *argp_program_version = "1.0.6";
 const char *argp_program_bug_address = "<carl@uvos.xyz>";
 static char doc[] = "Deamon that stops programms via SIGSTOP when their X11 windows lose focus.";
 static char args_doc[] = "";
@@ -33,6 +34,7 @@ static char args_doc[] = "";
 static struct argp_option options[] = 
 {
   {"ignore-client-machine",  'i', 0,      0,  "Also stop programs associated with windows that fail to set WM_CLIENT_MACHINE" },
+  {"timout", 't', "seconds",      0,  "Timeout to give program to close its last window before stoping it" },
   { 0 }
 };
 
@@ -45,6 +47,9 @@ error_t parse_opt (int key, char *arg, struct argp_state *state)
         case 'i':
         config->ignoreClientMachine = true;
         break;
+        case 't':
+        config->timeoutSecs = atol(arg);
+        break;
         default:
         return ARGP_ERR_UNKNOWN;
     }

debian/changelog

@@ -1,3 +1,21 @@
+sigstoped (1.0.6) unstable; urgency=medium
+  Fix Timer overflow on 32bit devices
+-- Uvos <carl@uvos.xyz>  Sat, 04 Jul 2020 11:47:00 +0100
+
+sigstoped (1.0.5) unstable; urgency=medium
+  Configureable timout for applications to close their windows.
+  Fix 5sec stall when switching from an application to be stopped to an application that was stopped before.
+  Make Xinstance thread safe
+-- Uvos <carl@uvos.xyz>  Mon, 16 Jun 2020 09:47:00 +0100
+
+sigstoped (1.0.4) unstable; urgency=medium
+  Fix memory leak in XInstance::getTopLevelWindows()
+ -- Uvos <carl@uvos.xyz>  Mon, 16 Jun 2020 09:47:00 +0100
+
+sigstoped (1.0.3) unstable; urgency=medium
+  Ignore BadWindow errors caused by faulty __NET_ACTIVE_WINDOW events
+ -- Uvos <carl@uvos.xyz>  Mon, 15 Jun 2020 23:47:00 +0100
+
 sigstoped (1.0.2) unstable; urgency=medium
   Inital version
  -- Uvos <carl@uvos.xyz>  Mon, 10 Jun 2020 15:00:00 +0100

debian/copywrite

@@ -0,0 +1,2 @@
+Unless stated otherwise, all files are:
+Copyright 2020 Carl Klemm and are licensed under the GPLv3

main.cpp

@@ -39,23 +39,27 @@
 #include "split.h"
 #include "debug.h"
 #include "argpopt.h"
+#include "CppTimer.h"
 
 Window intraCommesWindow;
-XInstance xinstance;
-volatile bool stop = false;
 volatile bool configStale = false;
+XInstance xinstance;
 
 constexpr char configPrefix[] = "/.config/sigstoped/";
+constexpr char STOP_EVENT = 65;
+constexpr char PROC_STOP_EVENT = 1;
 
 void sigTerm(int dummy) 
 {
-    stop = true;
+    XClientMessageEvent event;
-    XClientMessageEvent dummyEvent;
+    memset(&event, 0, sizeof(XClientMessageEvent));
-    memset(&dummyEvent, 0, sizeof(XClientMessageEvent));
+    event.type = ClientMessage;
-    dummyEvent.type = ClientMessage;
+    event.window = intraCommesWindow;
-    dummyEvent.window = intraCommesWindow;
+    event.format = 8;
-    dummyEvent.format = 32;
+    event.data.b[0] = STOP_EVENT;
-    XSendEvent(xinstance.display, intraCommesWindow, 0, 0, (XEvent*)&dummyEvent);
+    XLockDisplay(xinstance.display);
+    XSendEvent(xinstance.display, intraCommesWindow, 0, 0, (XEvent*)&event);
+    XUnlockDisplay(xinstance.display);
     xinstance.flush();
 }
 
@@ -121,7 +125,7 @@ bool createPidFile(const std::string& fileName)
         {
             std::cerr<<"Only one "
                      <<sigstopedName
-                     <<" process exists, either sigstoped died or you have severl diferently named binarys\n";
+                     <<" process exists, either sigstoped died or you have several diferently named binarys\n";
                      
             std::filesystem::remove(fileName);
             return createPidFile(fileName);
@@ -145,11 +149,56 @@ bool createPidFile(const std::string& fileName)
     }
 }
 
+
+void sendEventProcStop()
+{
+    XClientMessageEvent event;
+    memset(&event, 0, sizeof(XClientMessageEvent));
+    event.type = ClientMessage;
+    event.window = intraCommesWindow;
+    event.format = 8;
+    event.data.b[0] = PROC_STOP_EVENT;
+    XLockDisplay(xinstance.display);
+    XSendEvent(xinstance.display, intraCommesWindow, 0, 0, (XEvent*)&event);
+    XUnlockDisplay(xinstance.display);
+    xinstance.flush();
+}
+
+bool stopProcess(Process process, XInstance* xinstance)
+{
+    bool hasTopLevelWindow = false;
+    std::vector<Window> tlWindows = xinstance->getTopLevelWindows();
+    for(auto& window : tlWindows) 
+    {
+        if(xinstance->getPid(window) == process.getPid()) hasTopLevelWindow = true;
+    }
+    if(hasTopLevelWindow)
+    {
+        process.stop(true);
+        std::cout<<"Stoping pid: "+std::to_string(process.getPid())+" name: "+process.getName()<<'\n';
+        return true;
+    }
+    else  
+    {
+        std::cout<<"not Stoping pid: "+std::to_string(process.getPid())+" name: "+process.getName()<<'\n';
+        return false;
+    }
+}
+
 int main(int argc, char* argv[])
 {
+    char* xDisplayName = std::getenv( "DISPLAY" );
+    if(xDisplayName == nullptr) 
+    {
+        std::cerr<<"DISPLAY enviroment variable must be set.\n";
+        return 1;
+    }
+
+    if(!xinstance.open(xDisplayName)) exit(1);
+    
     Config config;
     argp_parse(&argp, argc, argv, 0, 0, &config);
-    
+
     if(config.ignoreClientMachine)
     {
         std::cout<<"WARNING: Ignoring WM_CLIENT_MACHINE is dangerous and may cause sigstoped to stop random pids if remote windows are present"<<std::endl;
@@ -159,7 +208,7 @@ int main(int argc, char* argv[])
     std::string confDir = getConfdir();
     if(confDir.size() == 0) return 1;
     
-    if(!createPidFile(confDir+"pidfile"));
+    if(!createPidFile(confDir+"pidfile")) return 1;
     
     std::vector<std::string> applicationNames = getApplicationlist(confDir+"blacklist");
     
@@ -171,17 +220,8 @@ int main(int argc, char* argv[])
         return 1;
     }
     
-    char* xDisplayName = std::getenv( "DISPLAY" );
-    if(xDisplayName == nullptr) 
-    {
-        std::cerr<<"DISPLAY enviroment variable must be set.\n";
-        return 1;
-    }
-    
     std::list<Process> stoppedProcs;
     
-    if(!xinstance.open(xDisplayName)) exit(1);
-    
     intraCommesWindow = XCreateSimpleWindow(xinstance.display, 
                                             RootWindow(xinstance.display, xinstance.screen),
                                             10, 10, 10, 10, 0, 0, 0);
@@ -190,6 +230,7 @@ int main(int argc, char* argv[])
     
     XEvent event;
     Process prevProcess;
+    Process qeuedToStop;
     Window prevWindow = 0;
     
     signal(SIGINT, sigTerm);
@@ -197,11 +238,13 @@ int main(int argc, char* argv[])
     signal(SIGHUP, sigTerm);
     signal(SIGUSR1, sigUser1);
     
-    while(!stop)
+    CppTimer timer;
+    
+    while(true)
     {
         XNextEvent(xinstance.display, &event);
         if (event.type == DestroyNotify) break;
-        if (event.type == PropertyNotify && event.xproperty.atom == xinstance.atoms.netActiveWindow)
+        else if (event.type == PropertyNotify && event.xproperty.atom == xinstance.atoms.netActiveWindow)
         {
             Window wid = xinstance.getActiveWindow();
             if(wid != 0 && wid != prevWindow)
@@ -223,6 +266,12 @@ int main(int argc, char* argv[])
                     {
                         if(process.getName() == applicationNames[i] &&  wid != 0 && process.getPid() > 0 && process.getName() != "") 
                         {
+                            if(process == qeuedToStop)
+                            {
+                                std::cout<<"Canceling stop of wid: "+std::to_string(wid)+" pid: "+std::to_string(process.getPid())+" name: "+process.getName()<<'\n';
+                                timer.stop();
+                                qeuedToStop = Process();
+                            }
                             process.resume(true);
                             stoppedProcs.remove(process);
                             std::cout<<"Resumeing wid: "+std::to_string(wid)+" pid: "+std::to_string(process.getPid())+" name: "+process.getName()<<'\n';
@@ -232,20 +281,11 @@ int main(int argc, char* argv[])
                                 prevProcess.getName() != "" && 
                                 prevProcess.getPid() > 0) 
                         {
-                            sleep(5); //give the process some time to close its other windows
+                            timer.block();
-                            bool hasTopLevelWindow = false;
+                            std::cout<<"Will stop pid: "<<prevProcess.getPid()<<" name: "<<prevProcess.getName()<<'\n';
-                            std::vector<Window> tlWindows = xinstance.getTopLevelWindows();
+                            qeuedToStop = prevProcess;
-                            for(auto& window : tlWindows) 
+                            timer.start(config.timeoutSecs, 0, sendEventProcStop, CppTimer::ONESHOT);
-                            {
+                            stoppedProcs.push_back(prevProcess);
-                                if(xinstance.getPid(window) == prevProcess.getPid()) hasTopLevelWindow = true;
-                            }
-                            if(hasTopLevelWindow)
-                            {
-                                prevProcess.stop(true);
-                                std::cout<<"Stoping wid: "+std::to_string(prevWindow)+" pid: "+std::to_string(prevProcess.getPid())+" name: "+prevProcess.getName()<<'\n';
-                                stoppedProcs.push_back(prevProcess);
-                            }
-                            else  std::cout<<"not Stoping wid: "+std::to_string(prevWindow)+" pid: "+std::to_string(prevProcess.getPid())+" name: "+prevProcess.getName()<<'\n';
                         }
                     }
                 }
@@ -253,6 +293,16 @@ int main(int argc, char* argv[])
                 prevWindow = wid;
             }
         }
+        else if (event.type == ClientMessage && ((XClientMessageEvent*)&event)->window == intraCommesWindow)
+        {
+            XClientMessageEvent* clientEvent = (XClientMessageEvent*)&event;
+            if (clientEvent->data.b[0] == STOP_EVENT) break;
+            if (clientEvent->data.b[0] == PROC_STOP_EVENT)
+            {
+               stopProcess(qeuedToStop, &xinstance);
+               qeuedToStop = Process();
+            }
+        }
     }
     for(auto& process : stoppedProcs) process.resume(true);
     std::filesystem::remove(confDir+"pidfile");

process.cpp

@@ -42,11 +42,14 @@ pid_t Process::getPid()
 
 void Process::stop(bool children)
 {
-    kill(pid_, SIGSTOP);
+    if(pid_ > 0)
-    if(children)
     {
-        std::vector<Process> children = getChildren();
+        kill(pid_, SIGSTOP);
-        for(auto& child : children) child.stop(true);
+        if(children)
+        {
+            std::vector<Process> children = getChildren();
+            for(auto& child : children) child.stop(true);
+        }
     }
 }
 

xinstance.cpp

@@ -30,7 +30,8 @@ unsigned long XInstance::readProparty(Window wid, Atom atom, unsigned char** pro
     Atom returnedAtom;
     unsigned long nitems;
     unsigned long bytes_after;
-        
+    
+    XLockDisplay(display);
     int ret = XGetWindowProperty(
         display, 
         wid, 
@@ -42,6 +43,7 @@ unsigned long XInstance::readProparty(Window wid, Atom atom, unsigned char** pro
         &nitems, 
         &bytes_after,
         prop);
+    XUnlockDisplay(display);
     if (ret != Success) 
     {
         std::cerr<<"XGetWindowProperty failed!\n";
@@ -52,11 +54,12 @@ unsigned long XInstance::readProparty(Window wid, Atom atom, unsigned char** pro
     
 Atom XInstance::getAtom(const std::string& atomName)
 {
-    return XInternAtom(display, atomName.c_str(), true);
+    return XInternAtom(display, atomName.c_str(), true);;
 }
     
 bool XInstance::open(const std::string& xDisplayName)
 {
+    XInitThreads();
     display = XOpenDisplay(xDisplayName.c_str());
     if (display == nullptr) 
     {
@@ -96,7 +99,9 @@ Window XInstance::getActiveWindow()
     unsigned long length = readProparty(RootWindow(display, screen), atoms.netActiveWindow, &data, &format);
     Window wid = 0;
     if(format == 32 && length == 4)  wid = *reinterpret_cast<Window*>(data);
+    XLockDisplay(display);
     XFree(data);
+    XUnlockDisplay(display);
     return wid;
 }
 
@@ -106,33 +111,46 @@ std::vector<Window> XInstance::getTopLevelWindows()
     Window parent_return;
     Window* windows = nullptr;
     unsigned int nwindows;
+    XLockDisplay(display);
     XQueryTree(display, RootWindow(display, screen), &root_return, &parent_return, &windows, &nwindows);
-    
+    XUnlockDisplay(display);
     std::vector<Window> out;
     out.reserve(nwindows);
-    for(unsigned int i; i < nwindows; ++i)
+    for(unsigned int i = 0; i < nwindows; ++i)
     {
         out.push_back(windows[i]);
     }
+    XLockDisplay(display);
+    if(windows != nullptr) XFree(windows); 
+    XUnlockDisplay(display);
     return out;
 }
 
 void XInstance::flush()
 {
-     XFlush(display);
+    XLockDisplay(display);
+    XFlush(display);
+    XUnlockDisplay(display);
 }
 
 pid_t XInstance::getPid(Window wid)
 {
+    defaultHandler = XSetErrorHandler(ignoreErrorHandler);
     XTextProperty xWidHostNameTextProperty;
     bool ret;
-    ret = XGetTextProperty(display, wid, &xWidHostNameTextProperty, atoms.wmClientMachine); 
+    XLockDisplay(display);
+    ret = XGetTextProperty(display, wid, &xWidHostNameTextProperty, atoms.wmClientMachine);
+    XUnlockDisplay(display);
     if (!ret) 
     {
         char errorString[1024];
         XGetErrorText(display, ret, errorString, 1024);
         debug("XGetWMClientMachine failed! " + std::string(errorString));
-        if(!ignoreClientMachine) return -1;
+        if(!ignoreClientMachine) 
+        {
+            XSetErrorHandler(defaultHandler);
+            return -1;
+        }
     }
     char** xWidHostNameStringList = nullptr;
     int nStrings;
@@ -142,13 +160,21 @@ pid_t XInstance::getPid(Window wid)
         char errorString[1024];
         XGetErrorText(display, ret, errorString, 1024);
         debug("XTextPropertyToStringList failed! " + std::string(errorString));
-        if(!ignoreClientMachine) return -1;
+        if(!ignoreClientMachine) 
+        {
+            XSetErrorHandler(defaultHandler);
+            return -1;
+        }
     }
     char hostName[HOST_NAME_MAX+1]={0};
     if(gethostname(hostName, HOST_NAME_MAX) != 0)
     {
         debug("Can't get host name");
-        if(!ignoreClientMachine) return -1;
+        if(!ignoreClientMachine) 
+        {
+            XSetErrorHandler(defaultHandler);
+            return -1;
+        }
     }
     pid_t pid = -1;
     if(ignoreClientMachine || strcmp(hostName, xWidHostNameStringList[0]) == 0 )
@@ -165,10 +191,20 @@ pid_t XInstance::getPid(Window wid)
         debug("Window "+std::to_string(wid)+" is a remote window");
     }
     if(xWidHostNameStringList) XFreeStringList(xWidHostNameStringList);
+    XSetErrorHandler(defaultHandler);
     return pid;
 }
 
+int XInstance::ignoreErrorHandler(Display* display, XErrorEvent* xerror)
+{
+    std::cerr<<"Ignoring: error code"<<xerror->error_code<<" request code "<<xerror->request_code<<'\n'
+             <<"this error most likely occured because of a bug in your WM\n";
+    return 0;
+}
+
  XInstance::~XInstance()
  {
+     XLockDisplay(display);
      if(display) XCloseDisplay(display);
+     XUnlockDisplay(display);
  }

xinstance.h

@@ -34,6 +34,7 @@ class XInstance
 
 public:
     
+    inline static XErrorHandler defaultHandler;
     static constexpr unsigned long MAX_BYTES = 1048576;
     
     inline static bool ignoreClientMachine = false;
@@ -46,6 +47,7 @@ private:
     
     unsigned long readProparty(Window wid, Atom atom, unsigned char** prop, int* format);
     Atom getAtom(const std::string& atomName);
+    static int ignoreErrorHandler(Display* display, XErrorEvent* xerror);
     
 public: