Fix #1441

src/modules/clipboard/waylandclipboard.cpp

@@ -8,6 +8,7 @@
 #include <unistd.h>
 #include <cstdint>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <string_view>
 #include <unordered_set>
@@ -60,7 +61,14 @@ void DataReaderThread::removeTask(uint64_t token) {
 void DataReaderThread::realRun() {
     EventLoop loop;
     dispatcherToWorker_.attach(&loop);
-    loop.exec();
+    bool terminate = false;
+    {
+        std::lock_guard<std::mutex> lock(mutex_);
+        terminate = terminate_;
+    }
+    if (!terminate) {
+        loop.exec();
+    }
     dispatcherToWorker_.detach();
     FCITX_DEBUG() << "Ending DataReaderThread";
     tasks_.clear();
@@ -275,70 +283,93 @@ WaylandClipboard::WaylandClipboard(Clipboard *clipboard, std::string name,
     globalConn_ = display_->globalCreated().connect(
         [this](const std::string &interface, const std::shared_ptr<void> &ptr) {
             if (interface == wayland::ExtDataControlManagerV1::interface) {
-                if (ptr != ext_manager_) {
-                    deviceMap_.clear();
-                    ext_manager_ =
+                if (ptr != extManager_) {
+                    extDeviceMap_.clear();
+                    extManager_ =
                         display_->getGlobal<wayland::ExtDataControlManagerV1>();
                 }
-                refreshSeat();
+                parent_->instance()->eventDispatcher().schedule(
+                    [this]() { refreshSeat(); });
+                deferRefreshSeat();
             } else if (interface ==
                        wayland::ZwlrDataControlManagerV1::interface) {
-                if (ptr != wlr_manager_) {
-                    deviceMap_.clear();
-                    wlr_manager_ =
+                if (ptr != wlrManager_) {
+                    wlrDeviceMap_.clear();
+                    wlrManager_ =
                         display_
                             ->getGlobal<wayland::ZwlrDataControlManagerV1>();
                 }
-                refreshSeat();
+                deferRefreshSeat();
             } else if (interface == wayland::WlSeat::interface) {
-                refreshSeat();
+                deferRefreshSeat();
             }
         });
     globalRemoveConn_ = display_->globalRemoved().connect(
         [this](const std::string &interface, const std::shared_ptr<void> &ptr) {
-            if (interface == wayland::ZwlrDataControlManagerV1::interface) {
-                deviceMap_.clear();
-                if (wlr_manager_ == ptr) {
-                    wlr_manager_.reset();
+            if (interface == wayland::ExtDataControlManagerV1::interface) {
+                extDeviceMap_.clear();
+                if (extManager_ == ptr) {
+                    extManager_.reset();
+                }
+            } else if (interface ==
+                       wayland::ZwlrDataControlManagerV1::interface) {
+                wlrDeviceMap_.clear();
+                if (wlrManager_ == ptr) {
+                    wlrManager_.reset();
                 }
             } else if (interface == wayland::WlSeat::interface) {
-                deviceMap_.erase(static_cast<wayland::WlSeat *>(ptr.get()));
+                wlrDeviceMap_.erase(static_cast<wayland::WlSeat *>(ptr.get()));
+                extDeviceMap_.erase(static_cast<wayland::WlSeat *>(ptr.get()));
             }
         });
 
     if (auto manager =
             display_->getGlobal<wayland::ZwlrDataControlManagerV1>()) {
-        wlr_manager_ = std::move(manager);
+        wlrManager_ = std::move(manager);
     }
     refreshSeat();
 }
 
+void WaylandClipboard::deferRefreshSeat() {
+    // The initial global registration update is more likely happen in one
+    // message loop, so we defer so we can decide to only initialize ext or wlr.
+    parent_->instance()->eventDispatcher().scheduleWithContext(
+        watch(), [this]() { refreshSeat(); });
+}
+
 void WaylandClipboard::refreshSeat() {
-    if (!wlr_manager_ && !ext_manager_) {
+    if (!wlrManager_ && !extManager_) {
         return;
     }
 
     auto seats = display_->getGlobals<wayland::WlSeat>();
     for (const auto &seat : seats) {
-        if (deviceMap_.contains(seat.get())) {
-            continue;
-        }
-
-        if (ext_manager_) {
-            auto *device = ext_manager_->getDataDevice(seat.get());
-            deviceMap_.emplace(
+        if (extManager_) {
+            if (extDeviceMap_.contains(seat.get())) {
+                continue;
+            }
+            auto *device = extManager_->getDataDevice(seat.get());
+            extDeviceMap_.emplace(
                 seat.get(),
                 std::make_unique<DataDevice<wayland::ExtDataControlDeviceV1>>(
                     this, device));
             continue;
-        } else if (wlr_manager_) {
-            auto *device = wlr_manager_->getDataDevice(seat.get());
-            deviceMap_.emplace(
+        } else if (wlrManager_) {
+            if (wlrDeviceMap_.contains(seat.get())) {
+                continue;
+            }
+            auto *device = wlrManager_->getDataDevice(seat.get());
+            wlrDeviceMap_.emplace(
                 seat.get(),
                 std::make_unique<DataDevice<wayland::ZwlrDataControlDeviceV1>>(
                     this, device));
         }
     }
+
+    // If both are available, prefer ext.
+    if (extManager_ && wlrManager_) {
+        wlrDeviceMap_.clear();
+    }
 }
 
 void WaylandClipboard::setClipboard(const std::string &str, bool password) {

src/modules/clipboard/waylandclipboard.h

@@ -11,6 +11,7 @@
 #include <functional>
 #include <list>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <thread>
 #include <unordered_map>
@@ -75,6 +76,13 @@ class DataReaderThread {
 
     ~DataReaderThread() {
         if (thread_ && thread_->joinable()) {
+            {
+                std::lock_guard<std::mutex> lock(mutex_);
+                terminate_ = true;
+            }
+            // If dispatcher is not attched, the schedule will do nothing.
+            // But after attach, reader thread will check terminate_
+            // So it won't stuck forever.
             dispatcherToWorker_.schedule([this]() {
                 if (auto *loop = dispatcherToWorker_.eventLoop()) {
                     loop->exit();
@@ -109,6 +117,11 @@ class DataReaderThread {
     std::unique_ptr<std::thread> thread_;
     uint64_t nextId_ = 1;
 
+    // Accessed by both thread
+    std::mutex mutex_;
+    bool terminate_ = false;
+    // End Accessed by both thread
+
     // Value only read/write by the reader thread.
     EventDispatcher dispatcherToWorker_;
     std::unordered_map<uint64_t, DataOfferTask> tasks_;
@@ -176,7 +189,7 @@ class DataDevice : public DataDeviceInterface {
     std::list<ScopedConnection> conns_;
 };
 
-class WaylandClipboard {
+class WaylandClipboard : public TrackableObject<WaylandClipboard> {
 
 public:
     WaylandClipboard(Clipboard *clipboard, std::string name,
@@ -188,16 +201,23 @@ class WaylandClipboard {
     auto parent() const { return parent_; }
 
 private:
+    void deferRefreshSeat();
     void refreshSeat();
     Clipboard *parent_;
     std::string name_;
     wayland::Display *display_;
     ScopedConnection globalConn_;
     ScopedConnection globalRemoveConn_;
-    std::shared_ptr<wayland::ExtDataControlManagerV1> ext_manager_;
-    std::shared_ptr<wayland::ZwlrDataControlManagerV1> wlr_manager_;
-    std::unordered_map<wayland::WlSeat *, std::unique_ptr<DataDeviceInterface>>
-        deviceMap_;
+    std::shared_ptr<wayland::ExtDataControlManagerV1> extManager_;
+    std::shared_ptr<wayland::ZwlrDataControlManagerV1> wlrManager_;
+    std::unordered_map<
+        wayland::WlSeat *,
+        std::unique_ptr<DataDevice<wayland::ExtDataControlDeviceV1>>>
+        extDeviceMap_;
+    std::unordered_map<
+        wayland::WlSeat *,
+        std::unique_ptr<DataDevice<wayland::ZwlrDataControlDeviceV1>>>
+        wlrDeviceMap_;
 };
 
 } // namespace fcitx