Browse Source

x11display: fix Xlib thread safety problems

rdb 7 years ago
parent
commit
ff3d005230
1 changed files with 21 additions and 7 deletions
  1. 21 7
      panda/src/x11display/x11GraphicsWindow.cxx

+ 21 - 7
panda/src/x11display/x11GraphicsWindow.cxx

@@ -136,10 +136,11 @@ x11GraphicsWindow(GraphicsEngine *engine, GraphicsPipe *pipe,
  */
 x11GraphicsWindow::
 ~x11GraphicsWindow() {
-  pmap<Filename, X11_Cursor>::iterator it;
-
-  for (it = _cursor_filenames.begin(); it != _cursor_filenames.end(); it++) {
-    XFreeCursor(_display, it->second);
+  if (!_cursor_filenames.empty()) {
+    LightReMutexHolder holder(x11GraphicsPipe::_x_mutex);
+    for (auto item : _cursor_filenames) {
+      XFreeCursor(_display, item.second);
+    }
   }
 }
 
@@ -157,17 +158,21 @@ get_pointer(int device) const {
 
     result = _input_devices[device].get_pointer();
 
-    // We recheck this immediately to get the most up-to-date value.
-    if (device == 0 && !_dga_mouse_enabled && result._in_window) {
+    // We recheck this immediately to get the most up-to-date value, but we
+    // won't bother waiting for the lock if we can't.
+    if (device == 0 && !_dga_mouse_enabled && result._in_window &&
+        x11GraphicsPipe::_x_mutex.try_lock()) {
       XEvent event;
+      LightReMutexHolder holder(x11GraphicsPipe::_x_mutex);
       if (XQueryPointer(_display, _xwindow, &event.xbutton.root,
           &event.xbutton.window, &event.xbutton.x_root, &event.xbutton.y_root,
           &event.xbutton.x, &event.xbutton.y, &event.xbutton.state)) {
         double time = ClockObject::get_global_clock()->get_real_time();
         result._xpos = event.xbutton.x;
         result._ypos = event.xbutton.y;
-        ((GraphicsWindowInputDevice &)_input_devices[0]).set_pointer(result._in_window, result._xpos, result._ypos, time);
+        ((GraphicsWindowInputDevice &)_input_devices[0]).set_pointer_in_window(result._xpos, result._ypos, time);
       }
+      x11GraphicsPipe::_x_mutex.release();
     }
   }
   return result;
@@ -197,6 +202,7 @@ move_pointer(int device, int x, int y) {
     const MouseData &md = _input_devices[0].get_pointer();
     if (!md.get_in_window() || md.get_x() != x || md.get_y() != y) {
       if (!_dga_mouse_enabled) {
+        LightReMutexHolder holder(x11GraphicsPipe::_x_mutex);
         XWarpPointer(_display, None, _xwindow, 0, 0, 0, 0, x, y);
       }
       _input_devices[0].set_pointer_in_window(x, y);
@@ -532,6 +538,8 @@ set_properties_now(WindowProperties &properties) {
   x11GraphicsPipe *x11_pipe;
   DCAST_INTO_V(x11_pipe, _pipe);
 
+  LightReMutexHolder holder(x11GraphicsPipe::_x_mutex);
+
   // We're either going into or out of fullscreen, or are in fullscreen and
   // are changing the resolution.
   bool is_fullscreen = _properties.has_fullscreen() && _properties.get_fullscreen();
@@ -858,6 +866,7 @@ close_window() {
     _gsg.clear();
   }
 
+  LightReMutexHolder holder(x11GraphicsPipe::_x_mutex);
   if (_ic != (XIC)nullptr) {
     XDestroyIC(_ic);
     _ic = (XIC)nullptr;
@@ -916,6 +925,9 @@ open_window() {
     _properties.set_size(100, 100);
   }
 
+  // Make sure we are not making X11 calls from other threads.
+  LightReMutexHolder holder(x11GraphicsPipe::_x_mutex);
+
   if (_properties.get_fullscreen() && x11_pipe->_have_xrandr) {
     XRRScreenConfiguration* conf = _XRRGetScreenInfo(_display, x11_pipe->get_root());
     if (_orig_size_id == (SizeID) -1) {
@@ -1059,6 +1071,8 @@ open_window() {
  * If already_mapped is true, the window has already been mapped (manifested)
  * on the display.  This means we may need to use a different action in some
  * cases.
+ *
+ * Assumes the X11 lock is held.
  */
 void x11GraphicsWindow::
 set_wm_properties(const WindowProperties &properties, bool already_mapped) {