Various stability fixes

applications/display-server/connection.cpp

@@ -22,11 +22,12 @@ static int pidfd_open(pid_t pid, unsigned int flags){
     return syscall(SYS_pidfd_open, pid, flags);
 }
 
-Connection::Connection(QObject* parent, pid_t pid, pid_t pgid)
-: QObject(parent),
+Connection::Connection(pid_t pid, pid_t pgid)
+: QObject(),
   m_pid{pid},
   m_pgid{pgid},
-  m_closed{false}
+  m_closed{false},
+  pingId{0}
 {
     m_pidFd = pidfd_open(m_pid, 0);
     if(m_pidFd < 0){
@@ -68,6 +69,14 @@ Connection::Connection(QObject* parent, pid_t pid, pid_t pgid)
 
 Connection::~Connection(){
     close();
+    surfaces.clear();
+    if(m_notifier != nullptr){
+        m_notifier->deleteLater();
+        m_notifier = nullptr;
+    }
+    ::close(m_clientFd);
+    ::close(m_serverFd);
+    ::close(m_pidFd);
     O_DEBUG("Connection" << id() << "destroyed");
 }
 
@@ -147,19 +156,11 @@ void Connection::resume(){
 }
 
 void Connection::close(){
-    surfaces.clear();
     if(!m_closed.test_and_set()){
-        return;
-    }
-    emit finished();
-    blockSignals(true);
-    if(m_notifier != nullptr){
-        m_notifier->deleteLater();
-        m_notifier = nullptr;
+        m_pingTimer.stop();
+        m_notRespondingTimer.stop();
+        emit finished();
     }
-    ::close(m_clientFd);
-    ::close(m_serverFd);
-    ::close(m_pidFd);
 }
 
 std::shared_ptr<Surface> Connection::addSurface(int fd, QRect geometry, int stride, QImage::Format format){
@@ -218,8 +219,6 @@ void Connection::inputEvent(const input_event& event){
     }
 }
 
-static std::atomic_uint pingId = 0;
-
 void Connection::readSocket(){
     if(m_notifier == nullptr){
         O_DEBUG("Connection already closed, discarding data");
@@ -504,6 +503,11 @@ void Connection::readSocket(){
 }
 
 void Connection::notResponding(){
+    if(!isRunning()){
+        close();
+        m_pingTimer.stop();
+        return;
+    }
     O_WARNING("Connection failed to respond to ping in time:" << id());
     m_notRespondingTimer.start();
 }
@@ -535,6 +539,9 @@ void Connection::ack(Blight::message_ptr_t message, unsigned int size, Blight::d
 }
 
 void Connection::ping(){
+    if(!isRunning()){
+        return;
+    }
     Blight::header_t ping{
         .type = Blight::MessageType::Ping,
         .ackid = ++pingId,

applications/display-server/connection.h

@@ -9,6 +9,7 @@
 #include <linux/input.h>
 
 #include "surface.h"
+class Surface;
 
 #include "../../shared/liboxide/meta.h"
 
@@ -18,7 +19,7 @@ class Connection : public QObject {
     Q_CLASSINFO("D-Bus Interface", BLIGHT_SURFACE_INTERFACE)
 
 public:
-    Connection(QObject* parent, pid_t pid, pid_t pgid);
+    Connection(pid_t pid, pid_t pgid);
     ~Connection();
 
     QString id();
@@ -65,6 +66,6 @@ private slots:
     std::atomic_flag m_closed;
     QTimer m_notRespondingTimer;
     QTimer m_pingTimer;
-    unsigned int pingId;
+    std::atomic_uint pingId;
     void ack(Blight::message_ptr_t message, unsigned int size, Blight::data_t data);
 };

applications/display-server/dbusinterface.cpp

@@ -1,5 +1,6 @@
 #include "dbusinterface.h"
 #include "evdevhandler.h"
+#include "guithread.h"
 
 #include <QDBusConnection>
 #include <QDBusUnixFileDescriptor>
@@ -63,6 +64,18 @@ DbusInterface::DbusInterface(QObject* parent)
     );
 }
 
+
+DbusInterface* DbusInterface::singleton(){
+    static DbusInterface* instance = nullptr;
+    if(instance == nullptr){
+        instance = Oxide::dispatchToMainThread<DbusInterface*>([]{
+            O_DEBUG("Initializing DBus interface");
+            return new DbusInterface(qApp);
+        });
+    }
+    return instance;
+}
+
 int DbusInterface::pid(){ return qApp->applicationPid(); }
 
 QObject* DbusInterface::loadComponent(QString url, QString identifier, QVariantMap properties){
@@ -102,14 +115,25 @@ QObject* DbusInterface::loadComponent(QString url, QString identifier, QVariantM
     return object;
 }
 
+void DbusInterface::processClosingConnections(){
+    closingMutex.lock();
+    if(!closingConnections.isEmpty()){
+        O_DEBUG("Cleaning up old connections");
+        while(!closingConnections.isEmpty()){
+            closingConnections.takeFirst()->deleteLater();
+        }
+    }
+    closingMutex.unlock();
+}
+
 std::shared_ptr<Surface> DbusInterface::getSurface(QString identifier){
     for(auto connection : qAsConst(connections)){
         if(!connection->isRunning()){
             continue;
         }
         auto surface = connection->getSurface(identifier);
         if(surface != nullptr){
-            return std::shared_ptr(surface);
+            return surface;
         }
     }
     return nullptr;
@@ -227,7 +251,7 @@ void DbusInterface::setFlags(QString identifier, const QStringList& flags, QDBus
     }
 }
 
-std::shared_ptr<Connection> DbusInterface::focused(){ return m_focused; }
+Connection* DbusInterface::focused(){ return m_focused; }
 
 void DbusInterface::serviceOwnerChanged(const QString& name, const QString& oldOwner, const QString& newOwner){
     Q_UNUSED(oldOwner);
@@ -296,7 +320,7 @@ void DbusInterface::inputEvents(unsigned int device, const std::vector<input_eve
     }
 }
 
-std::shared_ptr<Connection> DbusInterface::getConnection(QDBusMessage message){
+Connection* DbusInterface::getConnection(QDBusMessage message){
     pid_t pid = connection().interface()->servicePid(message.service());;
     for(auto connection : qAsConst(connections)){
         if(!connection->isRunning()){
@@ -322,43 +346,46 @@ QObject* DbusInterface::workspace(){
     return nullptr;
 }
 
-std::shared_ptr<Connection> DbusInterface::createConnection(int pid){
+Connection* DbusInterface::createConnection(int pid){
     pid_t pgid = ::getpgid(pid);
-    auto connection = new Connection(this, pid, pgid);
+    auto connection = new Connection(pid, pgid);
     if(!connection->isValid()){
         connection->deleteLater();
         return nullptr;
     }
-    auto ptr = std::shared_ptr<Connection>(connection);
     connect(connection, &Connection::finished, this, [this, connection]{
         O_DEBUG("Connection" << connection->pid() << "closed");
         auto found = false;
         for(auto& ptr : qAsConst(connections)){
-            if(ptr.get() == connection){
+            if(ptr == connection){
                 found = true;
                 connections.removeAll(ptr);
+                closingMutex.lock();
+                closingConnections.append(ptr);
+                closingMutex.unlock();
+                guiThread->notify();
                 break;
             }
         }
         if(!found){
             O_WARNING("Could not find connection to remove!");
         }
-        if(m_focused != nullptr && m_focused.get() == connection){
+        if(m_focused != nullptr && m_focused == connection){
             m_focused = nullptr;
         }
         sortZ();
     });
     connect(connection, &Connection::focused, this, [this, connection]{
         for(auto& ptr : qAsConst(connections)){
-            if(ptr.get() == connection){
+            if(ptr == connection){
                 m_focused = ptr;
                 O_DEBUG(connection->id() << "has focus");
                 break;
             }
         }
     });
-    connections.append(ptr);
-    return ptr;
+    connections.append(connection);
+    return connection;
 }
 
 QList<std::shared_ptr<Surface>> DbusInterface::surfaces(){
@@ -429,9 +456,9 @@ void DbusInterface::sortZ(){
     if(surface == nullptr){
         return;
     }
-    auto connection = dynamic_cast<Connection*>(surface->parent());
+    auto connection = surface->connection();
     for(auto& ptr : connections){
-        if(ptr.get() == connection){
+        if(ptr == connection){
             m_focused = ptr;
             break;
         }

applications/display-server/dbusinterface.h

@@ -1,12 +1,12 @@
 #pragma once
 
 #include <QObject>
+#include <QMutex>
+#include <QTimer>
 #include <QDBusContext>
 #include <QDBusConnection>
-#pragma once
 #include <QDBusConnectionInterface>
 #include <QDBusMessage>
-#include <QTimer>
 #include <QQmlApplicationEngine>
 #include <QDBusUnixFileDescriptor>
 
@@ -25,22 +25,17 @@ class DbusInterface : public QObject, public QDBusContext {
     Q_PROPERTY(QByteArray selection READ selection WRITE setSelection NOTIFY selectionChanged)
 
 public:
-    static DbusInterface* singleton(){
-        static DbusInterface* instance = nullptr;
-        if(instance == nullptr){
-            instance = new DbusInterface(qApp);
-        }
-        return instance;
-    }
+    static DbusInterface* singleton();
 
     int pid();
     QObject* loadComponent(QString url, QString identifier, QVariantMap properties = QVariantMap());
+    void processClosingConnections();
     std::shared_ptr<Surface> getSurface(QString identifier);
     QList<std::shared_ptr<Surface>> surfaces();
     QList<std::shared_ptr<Surface>> sortedSurfaces();
     QList<std::shared_ptr<Surface>> visibleSurfaces();
     void sortZ();
-    std::shared_ptr<Connection> focused();
+    Connection* focused();
 
     // Property getter/setters
     const QByteArray& clipboard();
@@ -77,14 +72,16 @@ private slots:
 private:
     DbusInterface(QObject* parent);
     QQmlApplicationEngine engine;
-    QList<std::shared_ptr<Connection>> connections;
-    std::shared_ptr<Connection> m_focused;
+    QList<Connection*> connections;
+    QMutex closingMutex;
+    QList<Connection*> closingConnections;
+    Connection* m_focused;
     struct {
         QByteArray clipboard;
         QByteArray selection;
     } clipboards;
 
-    std::shared_ptr<Connection> getConnection(QDBusMessage message);
+    Connection* getConnection(QDBusMessage message);
     QObject* workspace();
-    std::shared_ptr<Connection> createConnection(int pid);
+    Connection* createConnection(int pid);
 };

applications/display-server/guithread.cpp

@@ -22,6 +22,9 @@ void GUIThread::run(){
             //        entire application
             m_repaintMutex.lock();
             if(!m_repaintCount.available()){
+                m_repaintMutex.unlock();
+                dbusInterface->processClosingConnections();
+                m_repaintMutex.lock();
                 m_repaintWait.wait(&m_repaintMutex);
             }
             // Surface is required as the callback may
@@ -102,10 +105,12 @@ void GUIThread::enqueue(
     });
     // Indicate that there is an item in the queue
     m_repaintCount.release();
-    m_repaintWait.notify_all();
+    notify();
     m_repaintMutex.unlock();
 }
 
+void GUIThread::notify(){ m_repaintWait.notify_all(); }
+
 void GUIThread::clearFrameBuffer(){
     EPFrameBuffer::instance()->framebuffer()->fill(Qt::white);
     EPFrameBuffer::sendUpdate(
@@ -189,7 +194,7 @@ void GUIThread::redraw(RepaintRequest& event){
             visibleSurfaces.begin(),
             visibleSurfaces.end(),
             [](std::shared_ptr<Surface> surface){
-                auto connection = dynamic_cast<Connection*>(surface->parent());
+                auto connection = surface->connection();
                 if(!surface->has("system") && getpgid(connection->pgid()) < 0){
                     O_WARNING(surface->id() << "With no running process");
                     return true;

applications/display-server/guithread.h

@@ -50,6 +50,7 @@ public slots:
         bool global = false,
         std::function<void()> callback = nullptr
     );
+    void notify();
     void clearFrameBuffer();
 
 private:

applications/display-server/surface.cpp

@@ -9,13 +9,13 @@
 #include <unistd.h>
 #include <liboxide/debug.h>
 
-Surface::Surface(QObject* parent, int fd, QRect geometry, int stride, QImage::Format format)
-: QObject(parent),
+Surface::Surface(Connection* connection, int fd, QRect geometry, int stride, QImage::Format format)
+: QObject(),
+  m_connection(connection),
   m_geometry(geometry),
   m_stride(stride),
   m_format(format)
 {
-    auto connection = dynamic_cast<Connection*>(parent);
     m_id = QString("%1/surface/%2").arg(connection->id()).arg(fd);
     if(!file.open(fd, QFile::ReadWrite, QFile::AutoCloseHandle)){
         O_WARNING("Failed to open file");
@@ -149,9 +149,11 @@ void Surface::set(const QString& flag){
 
 void Surface::unset(const QString& flag){ flags.removeAll(flag); }
 
+Connection* Surface::connection(){ return m_connection; }
+
 void Surface::activeFocusChanged(bool focus){
     if(focus){
-        emit dynamic_cast<Connection*>(parent())->focused();
+        emit m_connection->focused();
     }
 }