[Spice-commits] client/x11
Hans de Goede
jwrdegoede at kemper.freedesktop.org
Tue Oct 12 01:28:04 PDT 2010
client/x11/platform.cpp | 180 ++++++++++++++++++++++++++++++++++++++------
client/x11/red_drawable.cpp | 7 +
client/x11/red_window.cpp | 158 +++++++++++++++++++++++++++++++++-----
client/x11/x_icon.cpp | 13 +++
4 files changed, 314 insertions(+), 44 deletions(-)
New commits:
commit f8c6e1c42a16d6b00f058389ee79baef7c0ff5d4
Author: Hans de Goede <hdegoede at redhat.com>
Date: Mon Oct 11 17:16:39 2010 +0200
spicec-x11: Put locks around xlib calls which wait for a reply
Since libX11-1.3.4 the multi-threading handling code of libX11 has been
changed, see:
http://cgit.freedesktop.org/xorg/lib/libX11/commit/?id=933aee1d5c53b0cc7d608011a29188b594c8d70b
This causes several issues. One of them is the display inside the
client not getting updated when there are no XEvents being generated,
this is caused by the following part of the referenced commit message:
Caveats:
- If one thread is waiting for events and another thread tries to read a reply,
both will hang until an event arrives. Previously, if this happened it might
work sometimes, but otherwise would trigger either an assertion failure or
a permanent hang.
We were depending on the otherwise behavior and apparently were lucky.
This can be seen by starting F14 in runlevel 3 and then doing startx
and not touching the mouse / keyb afterwards. Once you move the mouse
(generate an event you see the UI contents being updated but not before.
Another thing both I and Alon (iirc) have seen are hangs where 2
threads are stuck in XSync waiting for a reply simultaneously. This might
be related to libxcb version, according to the libX11 commit a libxcb
newer then 1.6 was needed, and my system had 1.5 at the time I saw this
after updating to libxcb 1.7 I can no longer reproduce.
This patch tackles both problems (and is needed for the 1st one
indepently of the 2nd one possibly being fixed) by adding XLockDisplay
calls around all libX11 calls which wait for a reply or an event.
diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index ccae877..9751a30 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -65,7 +65,7 @@
#define BOOL bool
#include "named_pipe.h"
-//#define X_DEBUG_SYNC(display) XSync(display, False)
+//#define X_DEBUG_SYNC(display) do { XLockDisplay(display); XSync(display, False); XUnlockDisplay(display); } while(0)
#define X_DEBUG_SYNC(display)
#ifdef HAVE_XRANDR12
#define USE_XRANDR_1_2
@@ -175,10 +175,16 @@ static Platform::ClipboardListener* clipboard_listener = &default_clipboard_list
static const char *atom_name(Atom atom)
{
+ const char *name;
+
if (atom == None)
return "None";
- return XGetAtomName(x_display, atom);
+ XLockDisplay(x_display);
+ name = XGetAtomName(x_display, atom);
+ XUnlockDisplay(x_display);
+
+ return name;
}
static uint32_t get_clipboard_type(Atom target) {
@@ -265,6 +271,7 @@ XEventHandler::XEventHandler(Display& x_display, XContext& win_proc_context)
void XEventHandler::on_event()
{
+ XLockDisplay(x_display);
while (XPending(&_x_display)) {
XPointer proc_pointer;
XEvent event;
@@ -281,8 +288,11 @@ void XEventHandler::on_event()
if (XFindContext(&_x_display, event.xany.window, _win_proc_context, &proc_pointer)) {
THROW("no window proc");
}
+ XUnlockDisplay(x_display);
((XPlatform::win_proc_t)proc_pointer)(event);
+ XLockDisplay(x_display);
}
+ XUnlockDisplay(x_display);
}
Display* XPlatform::get_display()
@@ -315,6 +325,12 @@ XImage *XPlatform::create_x_shm_image(RedDrawable::Format format,
XImage *image;
XShmSegmentInfo *shminfo;
+ /* We need to lock the display early, and force any pending requests to
+ be processed, to make sure that any errors reported by
+ handle_x_errors_stop() are actually ours */
+ XLockDisplay(XPlatform::get_display());
+ XSync(XPlatform::get_display(), False);
+
shminfo = new XShmSegmentInfo;
shminfo->shmid = -1;
shminfo->shmaddr = NULL;
@@ -375,6 +391,8 @@ XImage *XPlatform::create_x_shm_image(RedDrawable::Format format,
the segment if the client crashes. */
shmctl(shminfo->shmid, IPC_RMID, 0);
+ XUnlockDisplay(XPlatform::get_display());
+
image->data = (char *)shminfo->shmaddr;
*shminfo_out = shminfo;
@@ -390,6 +408,7 @@ err2:
}
err1:
+ XUnlockDisplay(XPlatform::get_display());
delete shminfo;
return NULL;
}
@@ -422,6 +441,7 @@ XImage *XPlatform::create_x_image(RedDrawable::Format format,
THROW("Out of memory");
}
+ XLockDisplay(XPlatform::get_display());
if (format == RedDrawable::A1) {
image = XCreateImage(XPlatform::get_display(),
NULL, 1, XYBitmap,
@@ -431,6 +451,7 @@ XImage *XPlatform::create_x_image(RedDrawable::Format format,
visual, depth, ZPixmap,
0, (char *)data, width, height, 32, stride);
}
+ XUnlockDisplay(XPlatform::get_display());
return image;
}
@@ -440,6 +461,7 @@ void XPlatform::free_x_image(XImage *image,
XShmSegmentInfo *shminfo)
{
if (shminfo) {
+ XLockDisplay(XPlatform::get_display());
XShmDetach(XPlatform::get_display(), shminfo);
}
if (image) {
@@ -448,6 +470,7 @@ void XPlatform::free_x_image(XImage *image,
if (shminfo) {
XSync(XPlatform::get_display(), False);
shmdt(shminfo->shmaddr);
+ XUnlockDisplay(XPlatform::get_display());
delete shminfo;
}
}
@@ -477,14 +500,21 @@ XIC XPlatform::get_input_context()
void XPlatform::set_win_proc(Window win, win_proc_t proc)
{
- if (XSaveContext(x_display, win, win_proc_context, (XPointer)proc)) {
+ int res;
+
+ XLockDisplay(x_display);
+ res = XSaveContext(x_display, win, win_proc_context, (XPointer)proc);
+ XUnlockDisplay(x_display);
+ if (res) {
THROW("set win proc failed");
}
}
void XPlatform::cleare_win_proc(Window win)
{
+ XLockDisplay(x_display);
XDeleteContext(x_display, win, win_proc_context);
+ XUnlockDisplay(x_display);
}
void Platform::send_quit_request()
@@ -651,12 +681,15 @@ static void show_scren_info()
int maxWidth;
int maxHeight;
+ XLockDisplay(x_display);
AutoScreenRes res(XRRGetScreenResources(x_display, root_window));
+ XUnlockDisplay(x_display);
if (!res.valid()) {
throw Exception(fmt("%s: get screen resources failed") % __FUNCTION__);
}
+ XLockDisplay(x_display);
XRRGetScreenSizeRange(x_display, root_window, &minWidth, &minHeight,
&maxWidth, &maxHeight);
printf("screen: min %dx%d max %dx%d\n", minWidth, minHeight,
@@ -706,6 +739,7 @@ static void show_scren_info()
crtc_info->x, crtc_info->y,
crtc_info->width, crtc_info->height, crtc_info->mode);
}
+ XUnlockDisplay(x_display);
}
#endif
@@ -767,7 +801,10 @@ XScreen::XScreen(Display* display, int screen)
int root = RootWindow(display, screen);
XWindowAttributes attrib;
+
+ XLockDisplay(display);
XGetWindowAttributes(display, root, &attrib);
+ XUnlockDisplay(display);
_position.x = attrib.x;
_position.y = attrib.y;
@@ -837,6 +874,8 @@ static void intern_clipboard_atoms()
int i;
static bool interned = false;
if (interned) return;
+
+ XLockDisplay(x_display);
clipboard_prop = XInternAtom(x_display, "CLIPBOARD", False);
incr_atom = XInternAtom(x_display, "INCR", False);
multiple_atom = XInternAtom(x_display, "MULTIPLE", False);
@@ -844,6 +883,8 @@ static void intern_clipboard_atoms()
for(i = 0; i < utf8_atom_count; i++)
utf8_atoms[i] = XInternAtom(x_display, utf8_atom_names[i], False);
+ XUnlockDisplay(x_display);
+
interned = true;
}
@@ -856,7 +897,10 @@ DynamicScreen::DynamicScreen(Display* display, int screen, int& next_mon_id)
{
X_DEBUG_SYNC(display);
//FIXME: replace RootWindow() in other refs as well?
+ XLockDisplay(display);
platform_win = XCreateSimpleWindow(display, RootWindow(display, screen), 0, 0, 1, 1, 0, 0, 0);
+ XUnlockDisplay(display);
+
LOG_INFO("platform_win: %u", (unsigned int)platform_win);
intern_clipboard_atoms();
XSelectInput(display, platform_win, StructureNotifyMask);
@@ -907,7 +951,10 @@ void DynamicScreen::do_set_mode(int width, int height)
int num_sizes;
X_DEBUG_SYNC(get_display());
+
+ XLockDisplay(get_display());
XRRScreenSize* sizes = XRRSizes(get_display(), get_screen(), &num_sizes);
+ XUnlockDisplay(get_display());
typedef std::set<SizeInfo, SizeCompare> SizesSet;
SizesSet sizes_set;
@@ -932,7 +979,10 @@ void DynamicScreen::do_restore()
}
int num_sizes;
+ XLockDisplay(get_display());
XRRScreenSize* sizes = XRRSizes(get_display(), get_screen(), &num_sizes);
+ XUnlockDisplay(get_display());
+
for (int i = 0; i < num_sizes; i++) {
if (sizes[i].width == _saved_width && sizes[i].height == _saved_height) {
set_screen_size(i);
@@ -949,7 +999,11 @@ bool DynamicScreen::set_screen_size(int size_index)
Window root_window = RootWindow(get_display(), get_screen());
XRRScreenConfiguration* config;
- if (!(config = XRRGetScreenInfo(get_display(), root_window))) {
+ XLockDisplay(get_display());
+ config = XRRGetScreenInfo(get_display(), root_window);
+ XUnlockDisplay(get_display());
+
+ if (!config) {
LOG_WARN("get screen info failed");
return false;
}
@@ -965,7 +1019,11 @@ bool DynamicScreen::set_screen_size(int size_index)
X_DEBUG_SYNC(get_display());
int num_sizes;
+
+ XLockDisplay(get_display());
XRRScreenSize* sizes = XRRSizes(get_display(), get_screen(), &num_sizes);
+ XUnlockDisplay(get_display());
+
if (num_sizes <= size_index) {
THROW("invalid sizes size");
}
@@ -1091,10 +1149,13 @@ MultyMonScreen::MultyMonScreen(Display* display, int screen, int& next_mon_id)
{
X_DEBUG_SYNC(get_display());
Window root_window = RootWindow(display, screen);
+
+ XLockDisplay(display);
XRRGetScreenSizeRange(display, root_window, &_min_width, &_min_height,
&_max_width, &_max_height);
-
AutoScreenRes res(XRRGetScreenResources(display, root_window));
+ XUnlockDisplay(display);
+
if (!res.valid()) {
THROW("get screen resources failed");
}
@@ -1102,6 +1163,7 @@ MultyMonScreen::MultyMonScreen(Display* display, int screen, int& next_mon_id)
#ifdef SHOW_SCREEN_INFO
show_scren_info();
#endif
+ XLockDisplay(display);
try {
for (int i = 0; i < res->ncrtc; i++) {
AutoCrtcInfo crtc_info(XRRGetCrtcInfo(display, res.get(), res->crtcs[i]));
@@ -1126,12 +1188,17 @@ MultyMonScreen::MultyMonScreen(Display* display, int screen, int& next_mon_id)
_monitors.push_back(new XMonitor(*this, next_mon_id++, res->crtcs[i]));
}
+ XUnlockDisplay(display);
} catch (...) {
+ XUnlockDisplay(display);
monitors_cleanup();
throw;
}
+ XLockDisplay(display);
platform_win = XCreateSimpleWindow(display, RootWindow(display, screen), 0, 0, 1, 1, 0, 0, 0);
+ XUnlockDisplay(display);
+
LOG_INFO("platform_win: %u", (unsigned int)platform_win);
intern_clipboard_atoms();
XSelectInput(display, platform_win, StructureNotifyMask);
@@ -1851,13 +1918,18 @@ void XMonitor::update_position()
Display* display = _container.get_display();
X_DEBUG_SYNC(display);
Window root_window = RootWindow(display, _container.get_screen());
+
+ XLockDisplay(display);
AutoScreenRes res(XRRGetScreenResources(display, root_window));
+ XUnlockDisplay(display);
if (!res.valid()) {
THROW("get screen resources failed");
}
+ XLockDisplay(display);
AutoCrtcInfo crtc_info(XRRGetCrtcInfo(display, res.get(), _crtc));
+ XUnlockDisplay(display);
ASSERT(crtc_info->noutput);
@@ -1887,7 +1959,9 @@ void XMonitor::update_position()
//todo: set valid subpixel order in case all outputs share the same type
_subpixel_order = RED_SUBPIXEL_ORDER_UNKNOWN;
} else {
+ XLockDisplay(display);
AutoOutputInfo output_info(XRRGetOutputInfo(display, res.get(), crtc_info->outputs[0]));
+ XUnlockDisplay(display);
switch (output_info->subpixel_order) {
case SubPixelUnknown:
@@ -1924,8 +1998,10 @@ void XMonitor::update_position()
bool XMonitor::finde_mode_in_outputs(RRMode mode, int start_index, XRRScreenResources* res)
{
int i, j;
+ bool retval = true;
X_DEBUG_SYNC(_container.get_display());
+ XLockDisplay(_container.get_display());
for (i = start_index; i < _noutput; i++) {
AutoOutputInfo output_info(XRRGetOutputInfo(_container.get_display(), res, _outputs[i]));
for (j = 0; j < output_info->nmode; j++) {
@@ -1934,11 +2010,13 @@ bool XMonitor::finde_mode_in_outputs(RRMode mode, int start_index, XRRScreenReso
}
}
if (j == output_info->nmode) {
- return false;
+ retval = false;
+ break;
}
}
+ XUnlockDisplay(_container.get_display());
X_DEBUG_SYNC(_container.get_display());
- return true;
+ return retval;
}
bool XMonitor::finde_mode_in_clones(RRMode mode, XRRScreenResources* res)
@@ -1975,7 +2053,11 @@ XRRModeInfo* XMonitor::find_mode(int width, int height, XRRScreenResources* res)
typedef std::set<ModeInfo, ModeCompare> ModesSet;
ModesSet modes_set;
X_DEBUG_SYNC(_container.get_display());
+
+ XLockDisplay(_container.get_display());
AutoOutputInfo output_info(XRRGetOutputInfo(_container.get_display(), res, _outputs[0]));
+ XUnlockDisplay(_container.get_display());
+
for (int i = 0; i < output_info->nmode; i++) {
XRRModeInfo* mode_inf = find_mod(res, output_info->modes[i]);
if (mode_inf->width >= width && mode_inf->height >= height) {
@@ -2010,7 +2092,11 @@ void XMonitor::do_set_mode(int width, int height)
Display* display = _container.get_display();
X_DEBUG_SYNC(display);
Window root_window = RootWindow(display, _container.get_screen());
+
+ XLockDisplay(display);
AutoScreenRes res(XRRGetScreenResources(display, root_window));
+ XUnlockDisplay(display);
+
if (!res.valid()) {
THROW("get screen resource failed");
}
@@ -2041,7 +2127,11 @@ void XMonitor::disable()
Display* display = _container.get_display();
X_DEBUG_SYNC(display);
Window root_window = RootWindow(display, _container.get_screen());
+
+ XLockDisplay(display);
AutoScreenRes res(XRRGetScreenResources(display, root_window));
+ XUnlockDisplay(display);
+
if (!res.valid()) {
THROW("get screen resources failed");
}
@@ -2061,7 +2151,11 @@ void XMonitor::enable()
Display* display = _container.get_display();
X_DEBUG_SYNC(display);
Window root_window = RootWindow(display, _container.get_screen());
+
+ XLockDisplay(display);
AutoScreenRes res(XRRGetScreenResources(display, root_window));
+ XUnlockDisplay(display);
+
if (!res.valid()) {
THROW("get screen resources failed");
}
@@ -2343,7 +2437,7 @@ static int get_selection(XEvent &event, Atom type, Atom prop, int format,
{
Bool del = incr ? True: False;
Atom type_ret;
- int format_ret, ret_val = -1;
+ int res, format_ret, ret_val = -1;
unsigned long len, remain;
unsigned char *data = NULL;
@@ -2366,9 +2460,15 @@ static int get_selection(XEvent &event, Atom type, Atom prop, int format,
}
}
- if (XGetWindowProperty(x_display, platform_win, prop, 0,
- LONG_MAX, del, type, &type_ret, &format_ret, &len,
- &remain, &data) != Success) {
+ /* Warning we are running with the clipboard_lock held!! That is ok, as
+ there is no code holding XLockDisplay which calls code taking
+ the clipboard_lock!! */
+ XLockDisplay(x_display);
+ res = XGetWindowProperty(x_display, platform_win, prop, 0,
+ LONG_MAX, del, type, &type_ret, &format_ret, &len,
+ &remain, &data);
+ XUnlockDisplay(x_display);
+ if (res != Success) {
LOG_WARN("XGetWindowProperty failed");
goto exit;
}
@@ -2686,17 +2786,25 @@ static void root_win_proc(XEvent& event)
static void process_monitor_configure_events(Window root)
{
- XSync(x_display, False);
XEvent event;
+ XLockDisplay(x_display);
+ XSync(x_display, False);
+
while (XCheckTypedWindowEvent(x_display, root, ConfigureNotify, &event)) {
+ XUnlockDisplay(x_display);
root_win_proc(event);
+ XLockDisplay(x_display);
}
while (XCheckTypedWindowEvent(x_display, root, xrandr_event_base + RRScreenChangeNotify,
&event)) {
+ XUnlockDisplay(x_display);
root_win_proc(event);
+ XLockDisplay(x_display);
}
+
+ XUnlockDisplay(x_display);
}
static void cleanup(void)
@@ -2779,7 +2887,7 @@ static void init_xfixes()
XFixesQueryVersion(x_display, &major, &minor) && major >= 1;
}
-unsigned int get_modifier_mask(KeySym modifier)
+static unsigned int get_modifier_mask(KeySym modifier)
{
int mask = 0;
int i;
@@ -3028,7 +3136,9 @@ uint32_t Platform::get_keyboard_lock_modifiers()
XKeyboardState keyboard_state;
uint32_t modifiers = 0;
+ XLockDisplay(x_display);
XGetKeyboardControl(x_display, &keyboard_state);
+ XUnlockDisplay(x_display);
if (keyboard_state.led_mask & 0x01) {
modifiers |= CAPS_LOCK_MODIFIER;
@@ -3088,7 +3198,7 @@ void Platform::set_keyboard_lock_modifiers(uint32_t modifiers)
}
}
-uint32_t key_bit(char* keymap, int key, uint32_t bit)
+static uint32_t key_bit(char* keymap, int key, uint32_t bit)
{
KeyCode key_code = XKeysymToKeycode(x_display, key);
return (((keymap[key_code >> 3] >> (key_code & 7)) & 1) ? bit : 0);
@@ -3097,14 +3207,19 @@ uint32_t key_bit(char* keymap, int key, uint32_t bit)
uint32_t Platform::get_keyboard_modifiers()
{
char keymap[32];
+ uint32_t mods;
+ XLockDisplay(x_display);
XQueryKeymap(x_display, keymap);
- return key_bit(keymap, XK_Shift_L, L_SHIFT_MODIFIER) |
+ mods = key_bit(keymap, XK_Shift_L, L_SHIFT_MODIFIER) |
key_bit(keymap, XK_Shift_R, R_SHIFT_MODIFIER) |
key_bit(keymap, XK_Control_L, L_CTRL_MODIFIER) |
key_bit(keymap, XK_Control_R, R_CTRL_MODIFIER) |
key_bit(keymap, XK_Alt_L, L_ALT_MODIFIER) |
key_bit(keymap, XK_Alt_R, R_ALT_MODIFIER);
+ XUnlockDisplay(x_display);
+
+ return mods;
}
void Platform::reset_cursor_pos()
@@ -3258,7 +3373,9 @@ XLocalCursor::XLocalCursor(CursorData* cursor_data)
case SPICE_CURSOR_TYPE_COLOR8:
default:
LOG_WARN("unsupported cursor type %d", header.type);
+ XLockDisplay(x_display);
_handle = XCreateFontCursor(x_display, XC_arrow);
+ XUnlockDisplay(x_display);
delete[] cur_data;
return;
}
@@ -3284,22 +3401,25 @@ XLocalCursor::XLocalCursor(CursorData* cursor_data)
}
Window root_window = RootWindow(x_display, DefaultScreen(x_display));
- Pixmap pixmap = XCreatePixmap(x_display, root_window, header.width, header.height, 32);
-
XGCValues gc_vals;
gc_vals.function = GXcopy;
gc_vals.foreground = ~0;
gc_vals.background = 0;
gc_vals.plane_mask = AllPlanes;
+ XLockDisplay(x_display);
+ Pixmap pixmap = XCreatePixmap(x_display, root_window, header.width, header.height, 32);
GC gc = XCreateGC(x_display, pixmap, GCFunction | GCForeground | GCBackground | GCPlaneMask,
&gc_vals);
+
XPutImage(x_display, pixmap, gc, &image, 0, 0, 0, 0, header.width, header.height);
XFreeGC(x_display, gc);
XRenderPictFormat *xformat = XRenderFindStandardFormat(x_display, PictStandardARGB32);
Picture picture = XRenderCreatePicture(x_display, pixmap, xformat, 0, NULL);
_handle = XRenderCreateCursor(x_display, picture, header.hot_spot_x, header.hot_spot_y);
+ XUnlockDisplay(x_display);
+
XRenderFreePicture(x_display, picture);
XFreePixmap(x_display, pixmap);
delete[] cur_data;
@@ -3323,7 +3443,12 @@ LocalCursor* Platform::create_inactive_cursor()
class XDefaultCursor: public XBaseLocalCursor {
public:
- XDefaultCursor() { _handle = XCreateFontCursor(x_display, XC_top_left_arrow);}
+ XDefaultCursor()
+ {
+ XLockDisplay(x_display);
+ _handle = XCreateFontCursor(x_display, XC_top_left_arrow);
+ XUnlockDisplay(x_display);
+ }
};
LocalCursor* Platform::create_default_cursor()
@@ -3440,13 +3565,17 @@ bool Platform::on_clipboard_notify(uint32_t type, const uint8_t* data, int32_t s
bool Platform::on_clipboard_request(uint32_t type)
{
+ Window owner;
Lock lock(clipboard_lock);
Atom target = get_clipboard_target(type);
if (target == None)
return false;
- if (XGetSelectionOwner(x_display, clipboard_prop) == None) {
+ XLockDisplay(x_display);
+ owner = XGetSelectionOwner(x_display, clipboard_prop);
+ XUnlockDisplay(x_display);
+ if (owner == None) {
LOG_INFO("No owner for the selection");
return false;
}
@@ -3464,8 +3593,12 @@ bool Platform::on_clipboard_request(uint32_t type)
void Platform::on_clipboard_release()
{
XEvent event;
+ Window owner;
- if (XGetSelectionOwner(x_display, clipboard_prop) != platform_win) {
+ XLockDisplay(x_display);
+ owner = XGetSelectionOwner(x_display, clipboard_prop);
+ XUnlockDisplay(x_display);
+ if (owner != platform_win) {
LOG_INFO("Platform::on_clipboard_release() called while not selection owner");
return;
}
@@ -3477,11 +3610,16 @@ void Platform::on_clipboard_release()
/* Make sure we process the XFixesSetSelectionOwnerNotify event caused
by this, so we don't end up changing the clipboard owner to none, after
it has already been re-owned because this event is still pending. */
+ XLockDisplay(x_display);
XSync(x_display, False);
while (XCheckTypedEvent(x_display,
XFixesSelectionNotify + xfixes_event_base,
- &event))
+ &event)) {
+ XUnlockDisplay(x_display);
root_win_proc(event);
+ XLockDisplay(x_display);
+ }
+ XUnlockDisplay(x_display);
/* Note no need to do a set_clipboard_owner(owner_none) here, as that is
already done by processing the XFixesSetSelectionOwnerNotify event. */
diff --git a/client/x11/red_drawable.cpp b/client/x11/red_drawable.cpp
index 327028b..e7151ed 100644
--- a/client/x11/red_drawable.cpp
+++ b/client/x11/red_drawable.cpp
@@ -216,7 +216,6 @@ static inline void copy_to_drawable_from_pixmap(const RedDrawable_p* dest,
dest->source.x_drawable.gc, source->pixmap.x_image,
src_x, src_y, area.left + offset.x, area.top + offset.y,
area.right - area.left, area.bottom - area.top, false);
- XSync(XPlatform::get_display(), 0);
} else {
XPutImage(XPlatform::get_display(), dest->source.x_drawable.drawable,
dest->source.x_drawable.gc, source->pixmap.x_image, src_x,
@@ -242,7 +241,6 @@ static inline void copy_to_drawable_from_pixmap(const RedDrawable_p* dest,
dest->source.x_drawable.gc, image,
0, 0, area.left + offset.x, area.top + offset.y,
area.right - area.left, area.bottom - area.top, false);
- XSync(XPlatform::get_display(), 0);
} else {
XPutImage(XPlatform::get_display(), dest->source.x_drawable.drawable,
dest->source.x_drawable.gc, image,
@@ -252,6 +250,7 @@ static inline void copy_to_drawable_from_pixmap(const RedDrawable_p* dest,
free_temp_image(image, shminfo, pixman_image);
}
+ XFlush(XPlatform::get_display());
}
static inline void copy_to_x_drawable(const RedDrawable_p* dest,
@@ -628,6 +627,7 @@ static inline void fill_drawable(RedDrawable_p* dest, const SpiceRect& area, rgb
Drawable drawable = dest->source.x_drawable.drawable;
GC gc = dest->source.x_drawable.gc;
+ XLockDisplay(XPlatform::get_display());
Colormap color_map = DefaultColormap(XPlatform::get_display(),
DefaultScreen(XPlatform::get_display()));
XColor x_color;
@@ -639,6 +639,7 @@ static inline void fill_drawable(RedDrawable_p* dest, const SpiceRect& area, rgb
if (!XAllocColor(XPlatform::get_display(), color_map, &x_color)) {
LOG_WARN("color map failed");
}
+ XUnlockDisplay(XPlatform::get_display());
XGCValues gc_vals;
gc_vals.foreground = x_color.pixel;
@@ -724,6 +725,7 @@ static inline void frame_drawable(RedDrawable_p* dest, const SpiceRect& area, rg
Drawable drawable = dest->source.x_drawable.drawable;
GC gc = dest->source.x_drawable.gc;
+ XLockDisplay(XPlatform::get_display());
Colormap color_map = DefaultColormap(XPlatform::get_display(),
DefaultScreen(XPlatform::get_display()));
XColor x_color;
@@ -735,6 +737,7 @@ static inline void frame_drawable(RedDrawable_p* dest, const SpiceRect& area, rg
if (!XAllocColor(XPlatform::get_display(), color_map, &x_color)) {
LOG_WARN("color map failed");
}
+ XUnlockDisplay(XPlatform::get_display());
XGCValues gc_vals;
gc_vals.foreground = x_color.pixel;
diff --git a/client/x11/red_window.cpp b/client/x11/red_window.cpp
index 5a0886a..e7b6815 100644
--- a/client/x11/red_window.cpp
+++ b/client/x11/red_window.cpp
@@ -309,13 +309,17 @@ enum KbdKeyCode {
static void query_keyboard()
{
+ XLockDisplay(x_display);
XkbDescPtr kbd_desk = XkbGetKeyboard(x_display, XkbAllComponentsMask, XkbUseCoreKbd);
+ XUnlockDisplay(x_display);
if (!kbd_desk) {
LOG_WARN("get keyboard failed");
return;
}
+ XLockDisplay(x_display);
char* keycodes = XGetAtomName(x_display, kbd_desk->names->keycodes);
+ XUnlockDisplay(x_display);
if (keycodes) {
if (strstr(keycodes, "evdev")) {
@@ -642,7 +646,9 @@ static void init_key_table_0xfe()
static inline RedKey to_red_key_code(unsigned int keycode)
{
+ XLockDisplay(x_display);
KeySym sym = XKeycodeToKeysym(x_display, keycode, 0);
+ XUnlockDisplay(x_display);
RedKey red_key;
if (sym == NoSymbol) {
@@ -772,8 +778,12 @@ void RedWindow_p::win_proc(XEvent& event)
{
XPointer window_pointer;
RedWindow* red_window;
+ int res;
- if (XFindContext(x_display, event.xany.window, user_data_context, &window_pointer)) {
+ XLockDisplay(x_display);
+ res = XFindContext(x_display, event.xany.window, user_data_context, &window_pointer);
+ XUnlockDisplay(x_display);
+ if (res) {
THROW("no user data");
}
red_window = (RedWindow*)window_pointer;
@@ -795,7 +805,10 @@ void RedWindow_p::win_proc(XEvent& event)
case KeyRelease: {
RedKey key = to_red_key_code(event.xkey.keycode);
XEvent next_event;
- if (XCheckWindowEvent(x_display, red_window->_win, ~long(0), &next_event)) {
+ XLockDisplay(x_display);
+ Bool check = XCheckWindowEvent(x_display, red_window->_win, ~long(0), &next_event);
+ XUnlockDisplay(x_display);
+ if (check) {
XPutBackEvent(x_display, &next_event);
if ((next_event.type == KeyPress) &&
(event.xkey.keycode == next_event.xkey.keycode) &&
@@ -921,6 +934,8 @@ void RedWindow_p::win_proc(XEvent& event)
void RedWindow_p::sync(bool shadowed)
{
+ XEvent event;
+
if (shadowed) {
_ignore_foucs = true;
_ignore_pointer = true;
@@ -928,11 +943,16 @@ void RedWindow_p::sync(bool shadowed)
_shadow_pointer_state = _pointer_in_window;
_shadow_focus_event.xany.serial = 0;
}
+
+ XLockDisplay(x_display);
XSync(x_display, False);
- XEvent event;
while (XCheckWindowEvent(x_display, _win, ~long(0), &event)) {
+ XUnlockDisplay(x_display);
win_proc(event);
+ XLockDisplay(x_display);
}
+ XUnlockDisplay(x_display);
+
if (!shadowed) {
return;
}
@@ -954,11 +974,17 @@ void RedWindow_p::wait_for_reparent()
{
XEvent event;
for (int i = 0; i < 50; i++) {
- if (XCheckTypedWindowEvent(x_display, _win, ReparentNotify, &event)) {
+ XLockDisplay(x_display);
+ bool check = XCheckTypedWindowEvent(x_display, _win, ReparentNotify, &event);
+ XUnlockDisplay(x_display);
+ if (check) {
return;
}
usleep(20 * 1000);
+ // HDG: why?? this makes no sense
+ XLockDisplay(x_display);
XSync(x_display, False);
+ XUnlockDisplay(x_display);
}
DBG(0, "failed");
}
@@ -968,7 +994,9 @@ void RedWindow_p::wait_for_map()
bool wait_parent = _expect_parent;
while (!_visibale) {
XEvent event;
+ XLockDisplay(x_display);
XWindowEvent(x_display, _win, ~0, &event);
+ XUnlockDisplay(x_display);
switch (event.type) {
case ReparentNotify:
wait_parent = false;
@@ -993,7 +1021,9 @@ void RedWindow_p::wait_for_unmap()
bool wait_parent = _expect_parent;
while (_visibale) {
XEvent event;
+ XLockDisplay(x_display);
XWindowEvent(x_display, _win, ~0, &event);
+ XUnlockDisplay(x_display);
switch (event.type) {
case ReparentNotify:
wait_parent = false;
@@ -1015,7 +1045,9 @@ void RedWindow_p::wait_for_unmap()
void RedWindow_p::set_glx(int width, int height)
{
if (_glcont_copy) {
+ XLockDisplay(x_display);
XSync(x_display, False);
+ XUnlockDisplay(x_display);
glXMakeCurrent(x_display, _win, _glcont_copy);
//glDrawBuffer(GL_FRONT);
glMatrixMode(GL_PROJECTION);
@@ -1050,8 +1082,10 @@ Cursor RedWindow_p::create_invisible_cursor(Window window)
XColor color;
char data[1] = {0};
Window root_window = RootWindow(x_display, DefaultScreen(x_display));
+ XLockDisplay(x_display);
Pixmap blank = XCreateBitmapFromData(x_display, root_window, data, 1, 1);
Cursor cursor = XCreatePixmapCursor(x_display, blank, blank, &color, &color, 0, 0);
+ XUnlockDisplay(x_display);
XFreePixmap(x_display, blank);
return cursor;
}
@@ -1071,6 +1105,8 @@ RedWindow_p::RedWindow_p()
void RedWindow_p::destroy(RedWindow& red_window, PixelsSource_p& pix_source)
{
+ XEvent event;
+
if (_win == None) {
return;
}
@@ -1082,9 +1118,12 @@ void RedWindow_p::destroy(RedWindow& red_window, PixelsSource_p& pix_source)
XPlatform::cleare_win_proc(_win);
XSelectInput(x_display, _win, 0);
+
+ XLockDisplay(x_display);
XSync(x_display, False);
- XEvent event;
while (XCheckWindowEvent(x_display, _win, ~long(0), &event));
+ XUnlockDisplay(x_display);
+
Window window = _win;
_win = None;
XFreeCursor(x_display, _invisible_cursor);
@@ -1123,6 +1162,7 @@ void RedWindow_p::create(RedWindow& red_window, PixelsSource_p& pix_source, int
ButtonReleaseMask | PointerMotionMask | FocusChangeMask |
EnterWindowMask | LeaveWindowMask;
+ XLockDisplay(x_display);
_colormap = XCreateColormap(x_display, root_window, XPlatform::get_vinfo()[in_screen]->visual,
AllocNone);
win_attributes.colormap = _colormap;
@@ -1131,22 +1171,34 @@ void RedWindow_p::create(RedWindow& red_window, PixelsSource_p& pix_source, int
width, height, 0, XPlatform::get_vinfo()[in_screen]->depth,
InputOutput, XPlatform::get_vinfo()[in_screen]->visual, mask,
&win_attributes);
+ XUnlockDisplay(x_display);
if (!window) {
THROW("create X window failed");
}
try {
- if (XSaveContext(x_display, window, user_data_context, (XPointer)&red_window)) {
+ int res;
+
+ XLockDisplay(x_display);
+ res = XSaveContext(x_display, window, user_data_context, (XPointer)&red_window);
+ XUnlockDisplay(x_display);
+ if (res) {
THROW("set win usr data failed");
}
XSetWMProtocols(x_display, window, &wm_delete_window_atom, 1);
XGCValues gc_vals;
- if (!(gc = XCreateGC(x_display, window, 0, &gc_vals))) {
+
+ XLockDisplay(x_display);
+ gc = XCreateGC(x_display, window, 0, &gc_vals);
+ XUnlockDisplay(x_display);
+ if (!gc) {
THROW("create gc failed");
}
- if (!(cursor = create_invisible_cursor(window))) {
+
+ cursor = create_invisible_cursor(window);
+ if (!cursor) {
THROW("create invisible cursor failed");
}
@@ -1189,7 +1241,9 @@ void RedWindow_p::migrate(RedWindow& red_window, PixelsSource_p& pix_source, int
THROW("get attributes failed");
}
XTextProperty text_pro;
+ XLockDisplay(x_display);
bool valid_title = XGetWMName(x_display, _win, &text_pro) && text_pro.value;
+ XUnlockDisplay(x_display);
destroy(red_window, pix_source);
create(red_window, pix_source, _show_pos.x, _show_pos.y, attrib.width, attrib.height,
to_screen);
@@ -1213,6 +1267,7 @@ void RedWindow_p::move_to_current_desktop()
unsigned char *prop_return;
long desktop = ~long(0);
+ XLockDisplay(x_display);
if (XGetWindowProperty(x_display, root, wm_current_desktop, 0, 1, False, AnyPropertyType,
&actual_type_return, &actual_format_return, &nitems_return,
&bytes_after_return, &prop_return) == Success &&
@@ -1221,6 +1276,7 @@ void RedWindow_p::move_to_current_desktop()
} else {
DBG(0, "get current desktop failed");
}
+ XUnlockDisplay(x_display);
XEvent xevent;
xevent.type = ClientMessage;
@@ -1266,7 +1322,9 @@ void RedWindow::set_title(std::wstring& title)
wchar_t *name = const_cast<wchar_t *>(title.c_str());
int r;
if (_win) {
+ XLockDisplay(x_display);
r = XwcTextListToTextProperty(x_display, &name, 1, XStringStyle, &text_prop);
+ XUnlockDisplay(x_display);
if (r >= 0) {
XSetWMName(x_display, _win, &text_prop);
XFree(text_prop.value);
@@ -1323,15 +1381,19 @@ public:
AutoXErrorHandler()
{
ASSERT(old_error_handler == NULL);
+ XLockDisplay(x_display);
XSync(x_display, False);
x_error = Success;
old_error_handler = XSetErrorHandler(x_error_handler);
+ XUnlockDisplay(x_display);
}
~AutoXErrorHandler()
{
if (old_error_handler) {
+ XLockDisplay(x_display);
XSetErrorHandler(old_error_handler);
+ XUnlockDisplay(x_display);
old_error_handler = NULL;
}
}
@@ -1344,8 +1406,12 @@ static Window get_window_for_reposition(Window window)
Window parent;
Window* childrens;
unsigned int num_childrens;
+ int res;
- if (!XQueryTree(x_display, window, &root, &parent, &childrens, &num_childrens)) {
+ XLockDisplay(x_display);
+ res = XQueryTree(x_display, window, &root, &parent, &childrens, &num_childrens);
+ XUnlockDisplay(x_display);
+ if (!res) {
return None;
}
@@ -1464,7 +1530,9 @@ static bool get_prop_32(Window win, Atom prop, uint32_t &val)
unsigned long bytes_after_return;
unsigned long nitems_return;
unsigned char *prop_return;
+ bool retval = false;
+ XLockDisplay(x_display);
if (XGetWindowProperty(x_display, win, prop, 0, 1, False, AnyPropertyType,
&actual_type_return, &actual_format_return,
&nitems_return, &bytes_after_return, &prop_return) == Success &&
@@ -1472,9 +1540,11 @@ static bool get_prop_32(Window win, Atom prop, uint32_t &val)
actual_type_return != None &&
actual_format_return == 32) {
val = *(uint32_t *)prop_return;
- return true;
+ retval = true;
}
- return false;
+ XUnlockDisplay(x_display);
+
+ return retval;
}
void RedWindow::external_show()
@@ -1486,7 +1556,9 @@ void RedWindow::external_show()
raise();
activate();
+ XLockDisplay(x_display);
atom = XInternAtom(x_display, "_NET_ACTIVE_WINDOW", true);
+ XUnlockDisplay(x_display);
if (atom != None) {
Window root;
XEvent xev;
@@ -1595,14 +1667,21 @@ static bool __get_position(Window window, SpicePoint& pos)
Window parent;
Window* childrens;
unsigned int num_childrens;
+ int res;
- if (!XGetWindowAttributes(x_display, window, &attrib)) {
+ XLockDisplay(x_display);
+ res = XGetWindowAttributes(x_display, window, &attrib);
+ XUnlockDisplay(x_display);
+ if (!res) {
return false;
}
pos.x += attrib.x;
pos.y += attrib.y;
- if (!XQueryTree(x_display, window, &root, &parent, &childrens, &num_childrens)) {
+ XLockDisplay(x_display);
+ res = XQueryTree(x_display, window, &root, &parent, &childrens, &num_childrens);
+ XUnlockDisplay(x_display);
+ if (!res) {
return false;
}
@@ -1648,7 +1727,9 @@ void RedWindow::do_start_key_interception()
// LeaveNotify and EnterNotify.
ASSERT(_focused);
+ XLockDisplay(x_display);
XGrabKeyboard(x_display, _win, True, GrabModeAsync, GrabModeAsync, CurrentTime);
+ XUnlockDisplay(x_display);
sync(true);
_listener.on_start_key_interception();
_key_interception_on = true;
@@ -1656,7 +1737,9 @@ void RedWindow::do_start_key_interception()
void RedWindow::do_stop_key_interception()
{
+ XLockDisplay(x_display);
XUngrabKeyboard(x_display, CurrentTime);
+ XUnlockDisplay(x_display);
sync(true);
_key_interception_on = false;
_listener.on_stop_key_interception();
@@ -1715,18 +1798,24 @@ void RedWindow::hide_cursor()
void RedWindow::release_mouse()
{
+ XLockDisplay(x_display);
XUngrabPointer(x_display, CurrentTime);
+ XUnlockDisplay(x_display);
sync(true);
}
void RedWindow::cupture_mouse()
{
int grab_retries = MOUSE_GRAB_RETRIES;
+ XLockDisplay(x_display);
XSync(x_display, False);
+ XUnlockDisplay(x_display);
for (;; --grab_retries) {
+ XLockDisplay(x_display);
int result = XGrabPointer(x_display, _win, True, 0,
GrabModeAsync, GrabModeAsync,
_win, None, CurrentTime);
+ XUnlockDisplay(x_display);
if (result == GrabSuccess) {
break;
}
@@ -1748,7 +1837,9 @@ void RedWindow::set_mouse_position(int x, int y)
SpicePoint RedWindow::get_size()
{
XWindowAttributes attrib;
+ XLockDisplay(x_display);
XGetWindowAttributes(x_display, _win, &attrib);
+ XUnlockDisplay(x_display);
SpicePoint size;
size.x = attrib.width;
size.y = attrib.height;
@@ -1777,7 +1868,12 @@ static QRegion *get_visibale_region(Window window)
QRegion* region = new QRegion;
region_init(region);
XWindowAttributes attrib;
- if (!XGetWindowAttributes(x_display, window, &attrib)) {
+ int res;
+
+ XLockDisplay(x_display);
+ res = XGetWindowAttributes(x_display, window, &attrib);
+ XUnlockDisplay(x_display);
+ if (!res) {
return NULL;
}
@@ -1800,11 +1896,20 @@ static QRegion *get_visibale_region(Window window)
AutoXErrorHandler auto_error_handler;
for (;;) {
- FAIL_ON_BAD_WINDOW(!XQueryTree(x_display, window, &root, &parent, &childrens,
- &num_childrens),
+ int res;
+
+ XLockDisplay(x_display);
+ res = XQueryTree(x_display, window, &root, &parent, &childrens,
+ &num_childrens);
+ XUnlockDisplay(x_display);
+ FAIL_ON_BAD_WINDOW(!res,
"%s: query X tree failed", __FUNCTION__);
for (int i = num_childrens - 1; i >= 0 && childrens[i] != prev; i--) {
- FAIL_ON_BAD_WINDOW(!XGetWindowAttributes(x_display, childrens[i], &attrib),
+
+ XLockDisplay(x_display);
+ res = XGetWindowAttributes(x_display, childrens[i], &attrib);
+ XUnlockDisplay(x_display);
+ FAIL_ON_BAD_WINDOW(!res,
"%s: get win attributes failed", __FUNCTION__);
if (attrib.map_state == IsViewable) {
@@ -1821,7 +1926,10 @@ static QRegion *get_visibale_region(Window window)
XFree(childrens);
}
- FAIL_ON_BAD_WINDOW(!XGetWindowAttributes(x_display, window, &attrib),
+ XLockDisplay(x_display);
+ res = XGetWindowAttributes(x_display, window, &attrib);
+ XUnlockDisplay(x_display);
+ FAIL_ON_BAD_WINDOW(!res,
"%s: get win attributes failed", __FUNCTION__);
window_area_from_attributes(window_area, attrib);
region_offset(region, window_area.left, window_area.top);
@@ -1830,7 +1938,10 @@ static QRegion *get_visibale_region(Window window)
break;
}
- FAIL_ON_BAD_WINDOW(!XGetWindowAttributes(x_display, parent, &attrib),
+ XLockDisplay(x_display);
+ res = XGetWindowAttributes(x_display, parent, &attrib);
+ XUnlockDisplay(x_display);
+ FAIL_ON_BAD_WINDOW(!res,
"%s: get win attributes failed", __FUNCTION__);
window_area_from_attributes(window_area, attrib);
window_area.right -= window_area.left;
@@ -1907,10 +2018,13 @@ bool RedWindow::get_mouse_anchor_point(SpicePoint& pt)
#ifdef USE_OGL
RedGlContext RedWindow::create_context_gl()
{
+ RedGlContext *context = NULL;
if (XPlatform::get_fbconfig()[_screen]) {
- return glXCreateContext(x_display, XPlatform::get_vinfo()[_screen], NULL, GL_TRUE);
+ XLockDisplay(x_display);
+ context = glXCreateContext(x_display, XPlatform::get_vinfo()[_screen], NULL, GL_TRUE);
+ XUnlockDisplay(x_display);
}
- return NULL;
+ return context;
}
RedPbuffer RedWindow::create_pbuff(int width, int height)
@@ -1925,8 +2039,10 @@ RedPbuffer RedWindow::create_pbuff(int width, int height)
0, 0 };
fb_config = XPlatform::get_fbconfig();
+ XLockDisplay(XPlatform::get_display());
pbuff = glXCreatePbuffer(XPlatform::get_display(), fb_config[_screen][0],
pbuf_attr);
+ XUnlockDisplay(XPlatform::get_display());
return pbuff;
}
diff --git a/client/x11/x_icon.cpp b/client/x11/x_icon.cpp
index e59a030..937eecc 100644
--- a/client/x11/x_icon.cpp
+++ b/client/x11/x_icon.cpp
@@ -61,13 +61,19 @@ void XIcon::get_pixmaps(int screen, Pixmap& out_pixmap, Pixmap& out_mask)
Pixmap mask = None;
GC gc = NULL;
try {
+ XLockDisplay(x_display);
pixmap = XCreatePixmap(x_display, root_window, _raw_icon->width, _raw_icon->height, 24);
+ XUnlockDisplay(x_display);
if (pixmap == None) {
THROW("create pixmap failed");
}
XWindowAttributes attr;
+
+ XLockDisplay(x_display);
XGetWindowAttributes(x_display, root_window, &attr);
+ XUnlockDisplay(x_display);
+
XImage image;
memset(&image, 0, sizeof(image));
image.width = _raw_icon->width;
@@ -94,14 +100,18 @@ void XIcon::get_pixmaps(int screen, Pixmap& out_pixmap, Pixmap& out_mask)
gc_vals.foreground = ~0;
gc_vals.background = 0;
gc_vals.plane_mask = AllPlanes;
+
+ XLockDisplay(x_display);
gc = XCreateGC(x_display, pixmap, GCFunction | GCForeground | GCBackground | GCPlaneMask,
&gc_vals);
XPutImage(x_display, pixmap, gc, &image, 0, 0, 0, 0, image.width, image.height);
+ // HDG: why ?? XFlush should suffice
XSync(x_display, False);
XFreeGC(x_display, gc);
gc = NULL;
mask = XCreatePixmap(x_display, root_window, _raw_icon->width, _raw_icon->height, 1);
+ XUnlockDisplay(x_display);
if (mask == None) {
THROW("create mask failed");
}
@@ -121,10 +131,13 @@ void XIcon::get_pixmaps(int screen, Pixmap& out_pixmap, Pixmap& out_mask)
THROW("init image failed");
}
+ XLockDisplay(x_display);
gc = XCreateGC(x_display, mask, GCFunction | GCForeground | GCBackground | GCPlaneMask,
&gc_vals);
XPutImage(x_display, mask, gc, &image, 0, 0, 0, 0, image.width, image.height);
+ // HDG: why ?? XFlush should suffice
XSync(x_display, False);
+ XUnlockDisplay(x_display);
XFreeGC(x_display, gc);
} catch (...) {
if (gc) {
More information about the Spice-commits
mailing list