From 235abae13406e6d406cc47c1ee7c500bc5e36f7e Mon Sep 17 00:00:00 2001 From: binary1248 Date: Tue, 15 May 2018 23:28:32 +0200 Subject: [PATCH] Fixed the Unix clipboard implementation causing an abort due to internal data races in Xlib. --- include/SFML/Window/Clipboard.hpp | 19 +- src/SFML/Window/Unix/ClipboardImpl.cpp | 523 +++++++++++++++---------- src/SFML/Window/Unix/ClipboardImpl.hpp | 78 ++++ src/SFML/Window/Unix/WindowImplX11.cpp | 4 + 4 files changed, 420 insertions(+), 204 deletions(-) diff --git a/include/SFML/Window/Clipboard.hpp b/include/SFML/Window/Clipboard.hpp index 436da8f9..50ea8042 100644 --- a/include/SFML/Window/Clipboard.hpp +++ b/include/SFML/Window/Clipboard.hpp @@ -60,7 +60,12 @@ public: /// This function sets the content of the clipboard as a /// string. /// - /// \param text sf::String containing the data to be sent + /// \warning Due to limitations on some operating systems, + /// setting the clipboard contents is only + /// guaranteed to work if there is currently an + /// open window for which events are being handled. + /// + /// \param text sf::String containing the data to be sent /// to the clipboard /// //////////////////////////////////////////////////////////// @@ -80,6 +85,11 @@ public: /// sf::Clipboard provides an interface for getting and /// setting the contents of the system clipboard. /// +/// It is important to note that due to limitations on some +/// operating systems, setting the clipboard contents is +/// only guaranteed to work if there is currently an open +/// window for which events are being handled. +/// /// Usage example: /// \code /// // get the clipboard content as a string @@ -96,11 +106,12 @@ public: /// // Using Ctrl + V to paste a string into SFML /// if(event.key.control && event.key.code == sf::Keyboard::V) /// string = sf::Clipboard::getString(); +/// +/// // Using Ctrl + C to copy a string out of SFML +/// if(event.key.control && event.key.code == sf::Keyboard::C) +/// sf::Clipboard::setString("Hello World!"); /// } /// } -/// -/// // set the clipboard to a string -/// sf::Clipboard::setString("Hello World!"); /// \endcode /// /// \see sf::String, sf::Event diff --git a/src/SFML/Window/Unix/ClipboardImpl.cpp b/src/SFML/Window/Unix/ClipboardImpl.cpp index 32ef21bc..df2fd954 100644 --- a/src/SFML/Window/Unix/ClipboardImpl.cpp +++ b/src/SFML/Window/Unix/ClipboardImpl.cpp @@ -27,143 +27,20 @@ //////////////////////////////////////////////////////////// #include #include -#include -#include -#include -#include -#include +#include +#include +#include +#include namespace { -//////////////////////////////////////////////////////////// -void initClipboard(); -void* hostSelection(void*); - -sf::String string; - -pthread_mutex_t mutex; -pthread_t host_thread; - -bool is_fail = false; -bool is_init = false; -bool is_host = false; - -Display* display = NULL; -Window window = 0; - -Atom selection = 0; -Atom atom_targ = 0; -Atom atom_text = 0; -Atom utf8_text = 0; -int xa_string = 31; -int xa_atom = 4; - -//////////////////////////////////////////////////////////// -void initClipboard() -{ - is_init = true; - - display = XOpenDisplay(NULL); - int screen = DefaultScreen(display); - window = XCreateSimpleWindow(display, RootWindow(display, screen), - 0, 0, 1, 1, 0, BlackPixel(display, screen), WhitePixel(display, screen)); - - selection = XInternAtom(display, "CLIPBOARD", false); - atom_targ = XInternAtom(display, "TARGETS", false); - atom_text = XInternAtom(display, "TEXT", false); - utf8_text = XInternAtom(display, "UTF8_STRING", true); - - if(utf8_text == None) + // Filter the events received by windows (only allow those matching a specific window) + Bool checkEvent(::Display*, XEvent* event, XPointer userData) { - std::cerr << "UTF-8 format unavailable on clipboard." << std::endl; - utf8_text = xa_string; + // Just check if the event matches the window + return event->xany.window == reinterpret_cast< ::Window >(userData); } - - if(pthread_mutex_init(&mutex, NULL)) - { - is_fail = true; - std::cerr << "Unable to initialize mutex. Failed to initialize clipboard." << std::endl; - return; - } - - if(pthread_create(&host_thread, NULL, hostSelection, NULL)) - { - is_fail = true; - std::cerr << "Unable to create host thread. Failed to initialize clipboard." << std::endl; - return; - } -} - -//////////////////////////////////////////////////////////// -void* hostSelection(void*) -{ - while(true) - { - if(XPending(display) && is_host) - { - XEvent event; - - pthread_mutex_lock(&mutex); - XNextEvent(display, &event); - pthread_mutex_unlock(&mutex); - - switch(event.type) - { - case SelectionClear: - { - pthread_mutex_lock(&mutex); - is_host = false; - pthread_mutex_unlock(&mutex); - - break; - } - case SelectionRequest: - { - if(event.xselectionrequest.selection == selection) - { - XSelectionRequestEvent* sel_req_event = &event.xselectionrequest; - XSelectionEvent sel_event = {0}; - - int result = 0; - sel_event.type = SelectionNotify, - sel_event.display = sel_req_event->display, - sel_event.requestor = sel_req_event->requestor, - sel_event.selection = sel_req_event->selection, - sel_event.time = sel_req_event->time, - sel_event.target = sel_req_event->target, - sel_event.property = sel_req_event->property; - - std::basic_string str = string.toUtf8(); - - if(sel_event.target == atom_targ) - result = XChangeProperty(sel_event.display, sel_event.requestor, - sel_event.property, xa_atom, 32, PropModeReplace, - reinterpret_cast(&utf8_text), 1); - else if(sel_event.target == xa_string || sel_event.target == atom_text) - result = XChangeProperty(sel_event.display, sel_event.requestor, - sel_event.property, xa_string, 8, PropModeReplace, - reinterpret_cast(&str[0]), str.size()); - else if(sel_event.target == utf8_text) - result = XChangeProperty(sel_event.display, sel_event.requestor, - sel_event.property, utf8_text, 8, PropModeReplace, - reinterpret_cast(&str[0]), str.size()); - else - sel_event.property = None; - - if((result & 2) == 0) - XSendEvent(display, sel_event.requestor, 0, 0, - reinterpret_cast(&sel_event)); - } - break; - } - default: break; - } - } - else - sf::sleep(sf::milliseconds(20)); - } -} } namespace sf @@ -174,86 +51,332 @@ namespace priv //////////////////////////////////////////////////////////// String ClipboardImpl::getString() { - if(!is_init) - initClipboard(); - - if(is_fail || is_host) - return string; - - // Dangerous! Wipes all previous events! - XSync(display, true); - XConvertSelection(display, selection, utf8_text, atom_text, window, CurrentTime); - - XEvent event; - - pthread_mutex_lock(&mutex); - XNextEvent(display, &event); - pthread_mutex_unlock(&mutex); - - if(event.type == SelectionNotify) - { - if(event.xselection.selection != selection || event.xselection.target != utf8_text) - { - std::cerr << "Failed to convert selection." << std::endl; - return string; - } - - if(event.xselection.property) - { - Atom target; - int format; - unsigned long size; - unsigned long byte_left; - unsigned char* data; - - XGetWindowProperty(event.xselection.display, - event.xselection.requestor, event.xselection.property, - 0L, (~0L), false, AnyPropertyType, - &target, &format, &size, &byte_left, &data); - - if(target == utf8_text) - { - std::basic_string str(data, size); - string = sf::String::fromUtf8(str.begin(), str.end()); - - XFree(data); - } - - XDeleteProperty(event.xselection.display, event.xselection.requestor, event.xselection.property); - } - } - - return string; + return getInstance().getStringImpl(); } //////////////////////////////////////////////////////////// void ClipboardImpl::setString(const String& text) { - if(!is_init) - initClipboard(); + getInstance().setStringImpl(text); +} - if(is_fail) - return; - if(!is_host) +//////////////////////////////////////////////////////////// +void ClipboardImpl::processEvents() +{ + getInstance().processEventsImpl(); +} + + +//////////////////////////////////////////////////////////// +ClipboardImpl::ClipboardImpl() : +m_window (0), +m_requestResponded(false) +{ + // Open a connection with the X server + m_display = OpenDisplay(); + + // Get the atoms we need to make use of the clipboard + m_clipboard = getAtom("CLIPBOARD", false); + m_targets = getAtom("TARGETS", false); + m_text = getAtom("TEXT", false); + m_utf8String = getAtom("UTF8_STRING", true ); + m_targetProperty = getAtom("SFML_CLIPBOARD_TARGET_PROPERTY", false); + + // Create a hidden window that will broker our clipboard interactions with X + m_window = XCreateSimpleWindow(m_display, DefaultRootWindow(m_display), 0, 0, 1, 1, 0, 0, 0); + + // Register the events we are interested in + XSelectInput(m_display, m_window, SelectionNotify | SelectionClear | SelectionRequest); +} + + +//////////////////////////////////////////////////////////// +ClipboardImpl::~ClipboardImpl() +{ + // Destroy the window + if (m_window) { - XSetSelectionOwner(display, selection, window, CurrentTime); - - if(XGetSelectionOwner(display, selection) != window) - { - std::cerr << "Unable to get ownership of selection." << std::endl; - return; - } - - pthread_mutex_lock(&mutex); - is_host = true; - pthread_mutex_unlock(&mutex); + XDestroyWindow(m_display, m_window); + XFlush(m_display); } - pthread_mutex_lock(&mutex); - string = text; - pthread_mutex_unlock(&mutex); + // Close the connection with the X server + CloseDisplay(m_display); +} + + +//////////////////////////////////////////////////////////// +ClipboardImpl& ClipboardImpl::getInstance() +{ + static ClipboardImpl instance; + + return instance; +} + + +//////////////////////////////////////////////////////////// +String ClipboardImpl::getStringImpl() +{ + // Check if anybody owns the current selection + if (XGetSelectionOwner(m_display, m_clipboard) == None) + { + m_clipboardContents.clear(); + + return m_clipboardContents; + } + + // Process any already pending events + processEvents(); + + m_requestResponded = false; + + // Request the current selection to be converted to UTF-8 (or STRING + // if UTF-8 is not available) and written to our window property + XConvertSelection( + m_display, + m_clipboard, + (m_utf8String != None) ? m_utf8String : XA_STRING, + m_targetProperty, + m_window, + CurrentTime + ); + + Clock clock; + + // Wait for a response for up to 1000ms + while (!m_requestResponded && (clock.getElapsedTime().asMilliseconds() < 1000)) + processEvents(); + + // If no response was received within the time period, clear our clipboard contents + if (!m_requestResponded) + m_clipboardContents.clear(); + + return m_clipboardContents; +} + + +//////////////////////////////////////////////////////////// +void ClipboardImpl::setStringImpl(const String& text) +{ + m_clipboardContents = text; + + // Set our window as the current owner of the selection + XSetSelectionOwner(m_display, m_clipboard, m_window, CurrentTime); + + // Check if setting the selection owner was successful + if (XGetSelectionOwner(m_display, m_clipboard) != m_window) + err() << "Cannot set clipboard string: Unable to get ownership of X selection" << std::endl; +} + + +//////////////////////////////////////////////////////////// +void ClipboardImpl::processEventsImpl() +{ + XEvent event; + + // Pick out the events that are interesting for this window + while (XCheckIfEvent(m_display, &event, &checkEvent, reinterpret_cast(m_window))) + m_events.push_back(event); + + // Handle the events for this window that we just picked out + while (!m_events.empty()) + { + event = m_events.front(); + m_events.pop_front(); + processEvent(event); + } +} + + +//////////////////////////////////////////////////////////// +void ClipboardImpl::processEvent(XEvent& windowEvent) +{ + switch (windowEvent.type) + { + case SelectionClear: + { + // We don't have any resources we need to clean up + // when losing selection ownership so we don't do + // anything when we receive SelectionClear + // We will still respond to any future SelectionRequest + // events since doing so doesn't really do any harm + break; + } + case SelectionNotify: + { + // Notification that the current selection owner + // has responded to our request + + XSelectionEvent& selectionEvent = *reinterpret_cast(&windowEvent.xselection); + + m_clipboardContents.clear(); + + // If retrieving the selection fails or conversion is unsuccessful + // we leave the contents of the clipboard empty since we don't + // own it and we don't know what it could currently be + if ((selectionEvent.property == None) || (selectionEvent.selection != m_clipboard)) + break; + + Atom type; + int format; + unsigned long items; + unsigned long remainingBytes; + unsigned char* data = 0; + + // The selection owner should have wrote the selection + // data to the specified window property + int result = XGetWindowProperty( + m_display, + m_window, + m_targetProperty, + 0, + 0x7fffffff, + False, + AnyPropertyType, + &type, + &format, + &items, + &remainingBytes, + &data + ); + + if (result == Success) + { + // We don't support INCR for now + // It is very unlikely that this will be returned + // for purely text data transfer anyway + if (type != getAtom("INCR", false)) + { + // Only copy the data if the format is what we expect + if ((type == m_utf8String) && (format == 8)) + { + m_clipboardContents = String::fromUtf8(data, data + items); + } + else if ((type == XA_STRING) && (format == 8)) + { + // Convert from ANSI std::string to sf::String + m_clipboardContents = std::string(data, data + items); + } + } + + XFree(data); + + // The selection requestor must always delete the property themselves + XDeleteProperty(m_display, m_window, m_targetProperty); + } + + m_requestResponded = true; + + break; + } + case SelectionRequest: + { + // Respond to a request for our clipboard contents + XSelectionRequestEvent& selectionRequestEvent = *reinterpret_cast(&windowEvent.xselectionrequest); + + // Our reply + XSelectionEvent selectionEvent; + + selectionEvent.type = SelectionNotify; + selectionEvent.requestor = selectionRequestEvent.requestor; + selectionEvent.selection = selectionRequestEvent.selection; + selectionEvent.property = selectionRequestEvent.property; + selectionEvent.time = selectionRequestEvent.time; + + if (selectionRequestEvent.selection == m_clipboard) + { + if (selectionRequestEvent.target == m_targets) + { + // Respond to a request for our valid conversion targets + std::vector targets; + + targets.push_back(m_targets); + targets.push_back(m_text); + targets.push_back(XA_STRING); + + if (m_utf8String != None) + targets.push_back(m_utf8String); + + XChangeProperty( + m_display, + selectionRequestEvent.requestor, + selectionRequestEvent.property, + XA_ATOM, + 32, + PropModeReplace, + reinterpret_cast(&targets[0]), + targets.size() + ); + + // Notify the requestor that they can read the targets from their window property + selectionEvent.target = m_targets; + + XSendEvent(m_display, selectionRequestEvent.requestor, True, NoEventMask, reinterpret_cast(&selectionEvent)); + + break; + } + else if ((selectionRequestEvent.target == XA_STRING) || ((m_utf8String == None) && (selectionRequestEvent.target == m_text))) + { + // Respond to a request for conversion to a Latin-1 string + std::string data = m_clipboardContents.toAnsiString(); + + XChangeProperty( + m_display, + selectionRequestEvent.requestor, + selectionRequestEvent.property, + XA_STRING, + 8, + PropModeReplace, + reinterpret_cast(data.c_str()), + data.size() + ); + + // Notify the requestor that they can read the data from their window property + selectionEvent.target = XA_STRING; + + XSendEvent(m_display, selectionRequestEvent.requestor, True, NoEventMask, reinterpret_cast(&selectionEvent)); + + break; + } + else if ((m_utf8String != None) && ((selectionRequestEvent.target == m_utf8String) || (selectionRequestEvent.target == m_text))) + { + // Respond to a request for conversion to a UTF-8 string + // or an encoding of our choosing (we always choose UTF-8) + std::basic_string data = m_clipboardContents.toUtf8(); + + XChangeProperty( + m_display, + selectionRequestEvent.requestor, + selectionRequestEvent.property, + m_utf8String, + 8, + PropModeReplace, + reinterpret_cast(data.c_str()), + data.size() + ); + + // Notify the requestor that they can read the data from their window property + selectionEvent.target = m_utf8String; + + XSendEvent(m_display, selectionRequestEvent.requestor, True, NoEventMask, reinterpret_cast(&selectionEvent)); + + break; + } + } + + // Notify the requestor that we could not respond to their request + selectionEvent.target = selectionRequestEvent.target; + selectionEvent.property = None; + + XSendEvent(m_display, selectionRequestEvent.requestor, True, NoEventMask, reinterpret_cast(&selectionEvent)); + + break; + } + default: + break; + } } } // namespace priv diff --git a/src/SFML/Window/Unix/ClipboardImpl.hpp b/src/SFML/Window/Unix/ClipboardImpl.hpp index f4aca983..48ec7876 100644 --- a/src/SFML/Window/Unix/ClipboardImpl.hpp +++ b/src/SFML/Window/Unix/ClipboardImpl.hpp @@ -29,6 +29,8 @@ // Headers //////////////////////////////////////////////////////////// #include +#include +#include namespace sf @@ -67,6 +69,82 @@ public: /// //////////////////////////////////////////////////////////// static void setString(const String& text); + + //////////////////////////////////////////////////////////// + /// \brief Process pending events for the hidden clipboard window + /// + /// This function has to be called as part of normal window + /// event processing in order for our application to respond + /// to selection requests from other applications. + /// + //////////////////////////////////////////////////////////// + static void processEvents(); + +private: + + //////////////////////////////////////////////////////////// + /// \brief Constructor + /// + //////////////////////////////////////////////////////////// + ClipboardImpl(); + + //////////////////////////////////////////////////////////// + /// \brief Destructor + /// + //////////////////////////////////////////////////////////// + ~ClipboardImpl(); + + //////////////////////////////////////////////////////////// + /// \brief Get singleton instance + /// + /// \return Singleton instance + /// + //////////////////////////////////////////////////////////// + static ClipboardImpl& getInstance(); + + //////////////////////////////////////////////////////////// + /// \brief getString implementation + /// + /// \return Current content of the clipboard + /// + //////////////////////////////////////////////////////////// + String getStringImpl(); + + //////////////////////////////////////////////////////////// + /// \brief setString implementation + /// + /// \param text sf::String object containing the data to be sent to the clipboard + /// + //////////////////////////////////////////////////////////// + void setStringImpl(const String& text); + + //////////////////////////////////////////////////////////// + /// \brief processEvents implementation + /// + //////////////////////////////////////////////////////////// + void processEventsImpl(); + + //////////////////////////////////////////////////////////// + /// \brief Process an incoming event from the window + /// + /// \param windowEvent Event which has been received + /// + //////////////////////////////////////////////////////////// + void processEvent(XEvent& windowEvent); + + //////////////////////////////////////////////////////////// + // Member data + //////////////////////////////////////////////////////////// + ::Window m_window; ///< X identifier defining our window + ::Display* m_display; ///< Pointer to the display + Atom m_clipboard; ///< X Atom identifying the CLIPBOARD selection + Atom m_targets; ///< X Atom identifying TARGETS + Atom m_text; ///< X Atom identifying TEXT + Atom m_utf8String; ///< X Atom identifying UTF8_STRING + Atom m_targetProperty; ///< X Atom identifying our destination window property + String m_clipboardContents; ///< Our clipboard contents + std::deque m_events; ///< Queue we use to store pending events for this window + bool m_requestResponded; ///< Holds whether our selection request has been responded to or not }; } // namespace priv diff --git a/src/SFML/Window/Unix/WindowImplX11.cpp b/src/SFML/Window/Unix/WindowImplX11.cpp index e2c8d290..ee5bf356 100644 --- a/src/SFML/Window/Unix/WindowImplX11.cpp +++ b/src/SFML/Window/Unix/WindowImplX11.cpp @@ -26,6 +26,7 @@ // Headers //////////////////////////////////////////////////////////// #include +#include #include #include #include @@ -779,6 +780,9 @@ void WindowImplX11::processEvents() m_events.pop_front(); processEvent(event); } + + // Process clipboard window events + priv::ClipboardImpl::processEvents(); }