[client,x11] refactor locking

X11 and railWindows lock must be held at the same time to avoid
deadlocking.
This commit is contained in:
Armin Novak
2026-03-01 09:23:11 +01:00
parent 5b4fae7afb
commit 4ff57b68c2
5 changed files with 61 additions and 52 deletions

View File

@@ -411,7 +411,7 @@ static BOOL xf_event_Expose(xfContext* xfc, const XExposeEvent* event, BOOL app)
xfAppWindow* appWindow = xf_AppWindowFromX11Window(xfc, event->window);
if (appWindow)
xf_UpdateWindowArea(xfc, appWindow, x, y, w, h);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return TRUE;
@@ -441,7 +441,7 @@ BOOL xf_generic_MotionNotify_(xfContext* xfc, int x, int y, Window window, BOOL
{
/* make sure window exists */
xfAppWindow* appWindow = xf_AppWindowFromX11Window(xfc, window);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
if (!appWindow)
return TRUE;
@@ -549,7 +549,7 @@ BOOL xf_generic_ButtonEvent_(xfContext* xfc, int x, int y, int button, Window wi
{
/* make sure window exists */
xfAppWindow* appWindow = xf_AppWindowFromX11Window(xfc, window);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
if (!appWindow)
return TRUE;
@@ -714,7 +714,7 @@ static BOOL xf_event_FocusIn(xfContext* xfc, const XFocusInEvent* event, BOOL ap
*/
if (appWindow)
xf_rail_adjust_position(xfc, appWindow);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
xf_keyboard_focus_in(xfc);
@@ -774,7 +774,7 @@ static BOOL xf_event_ClientMessage(xfContext* xfc, const XClientMessageEvent* ev
if (appWindow)
rc = xf_rail_send_client_system_command(xfc, appWindow->windowId, SC_CLOSE);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
return rc;
}
else
@@ -808,7 +808,7 @@ static BOOL xf_event_EnterNotify(xfContext* xfc, const XEnterWindowEvent* event,
/* keep track of which window has focus so that we can apply pointer updates */
xfc->appWindow = appWindow;
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return TRUE;
@@ -830,7 +830,7 @@ static BOOL xf_event_LeaveNotify(xfContext* xfc, const XLeaveWindowEvent* event,
/* keep track of which window has focus so that we can apply pointer updates */
if (xfc->appWindow == appWindow)
xfc->appWindow = nullptr;
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return TRUE;
}
@@ -934,7 +934,7 @@ static BOOL xf_event_ConfigureNotify(xfContext* xfc, const XConfigureEvent* even
xf_rail_adjust_position(xfc, appWindow);
}
}
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return xf_pointer_update_scale(xfc);
}
@@ -961,7 +961,7 @@ static BOOL xf_event_MapNotify(xfContext* xfc, const XMapEvent* event, BOOL app)
// xf_rail_send_client_system_command(xfc, appWindow->windowId, SC_RESTORE);
appWindow->is_mapped = TRUE;
}
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return TRUE;
@@ -983,7 +983,7 @@ static BOOL xf_event_UnmapNotify(xfContext* xfc, const XUnmapEvent* event, BOOL
if (appWindow)
appWindow->is_mapped = FALSE;
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return TRUE;
@@ -1112,7 +1112,7 @@ static BOOL xf_event_PropertyNotify(xfContext* xfc, const XPropertyEvent* event,
rc = gdi_send_suppress_output(xfc->common.context.gdi, minimized);
fail:
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
return rc;
@@ -1232,7 +1232,7 @@ BOOL xf_event_process(freerdp* instance, const XEvent* event)
xfc->appWindow = appWindow;
const BOOL rc = xf_event_suppress_events(xfc, appWindow, event);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
if (rc)
return TRUE;
}

View File

@@ -75,6 +75,7 @@ static UINT xf_OutputUpdate(xfContext* xfc, xfGfxSurface* surface)
if (!(rects = region16_rects(&surface->gdi.invalidRegion, &nbRects)))
return CHANNEL_RC_OK;
xf_lock_x11(xfc);
for (UINT32 x = 0; x < nbRects; x++)
{
const RECTANGLE_16* rect = &rects[x];
@@ -102,9 +103,7 @@ static UINT xf_OutputUpdate(xfContext* xfc, xfGfxSurface* surface)
WINPR_ASSERTING_INT_CAST(int, nYSrc),
WINPR_ASSERTING_INT_CAST(int, nXDst),
WINPR_ASSERTING_INT_CAST(int, nYDst), dwidth, dheight);
xf_lock_x11(xfc);
xf_rail_paint_surface(xfc, surface->gdi.windowId, rect);
xf_unlock_x11(xfc);
}
else
#ifdef WITH_XRENDER
@@ -137,6 +136,7 @@ fail:
region16_clear(&surface->gdi.invalidRegion);
LogDynAndXSetClipMask(xfc->log, xfc->display, xfc->gc, None);
LogDynAndXSync(xfc->log, xfc->display, False);
xf_unlock_x11(xfc);
return rc;
}
@@ -473,10 +473,10 @@ static UINT xf_UnmapWindowForSurface(RdpgfxClientContext* context, UINT64 window
if (freerdp_settings_get_bool(gdi->context->settings, FreeRDP_RemoteApplicationMode))
{
xfAppWindow* appWindow = xf_rail_get_window(xfc, windowID);
xfAppWindow* appWindow = xf_rail_get_window(xfc, windowID, FALSE);
if (appWindow)
xf_AppWindowDestroyImage(appWindow);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
}
WLog_WARN(TAG, "function not implemented");

View File

@@ -159,7 +159,7 @@ BOOL xf_rail_send_activate(xfContext* xfc, Window xwindow, BOOL enabled)
WINPR_ASSERT(appWindow->windowId <= UINT32_MAX);
activate.windowId = (UINT32)appWindow->windowId;
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
activate.enabled = enabled;
const UINT rc = xfc->rail->ClientActivate(xfc->rail, &activate);
@@ -293,7 +293,7 @@ BOOL xf_rail_end_local_move(xfContext* xfc, xfAppWindow* appWindow)
BOOL xf_rail_paint_surface(xfContext* xfc, UINT64 windowId, const RECTANGLE_16* rect)
{
xfAppWindow* appWindow = xf_rail_get_window(xfc, windowId);
xfAppWindow* appWindow = xf_rail_get_window(xfc, windowId, FALSE);
WINPR_ASSERT(rect);
@@ -329,7 +329,7 @@ BOOL xf_rail_paint_surface(xfContext* xfc, UINT64 windowId, const RECTANGLE_16*
updateRect.right - updateRect.left, updateRect.bottom - updateRect.top);
}
region16_uninit(&windowInvalidRegion);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
return TRUE;
}
@@ -391,7 +391,7 @@ static BOOL xf_rail_window_common(rdpContext* context, const WINDOW_ORDER_INFO*
UINT32 fieldFlags = orderInfo->fieldFlags;
BOOL position_or_size_updated = FALSE;
xfAppWindow* appWindow = xf_rail_get_window(xfc, orderInfo->windowId);
xfAppWindow* appWindow = xf_rail_get_window(xfc, orderInfo->windowId, FALSE);
if (fieldFlags & WINDOW_ORDER_STATE_NEW)
{
@@ -401,7 +401,7 @@ static BOOL xf_rail_window_common(rdpContext* context, const WINDOW_ORDER_INFO*
windowState->windowHeight, 0xFFFFFFFF);
if (!appWindow)
return FALSE;
goto fail;
appWindow->dwStyle = windowState->style;
appWindow->dwExStyle = windowState->extendedStyle;
@@ -679,7 +679,7 @@ static BOOL xf_rail_window_common(rdpContext* context, const WINDOW_ORDER_INFO*
}*/
rc = TRUE;
fail:
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
return rc;
}
@@ -829,7 +829,7 @@ static BOOL xf_rail_window_icon(rdpContext* context, const WINDOW_ORDER_INFO* or
BOOL rc = FALSE;
xfContext* xfc = (xfContext*)context;
BOOL replaceIcon = 0;
xfAppWindow* railWindow = xf_rail_get_window(xfc, orderInfo->windowId);
xfAppWindow* railWindow = xf_rail_get_window(xfc, orderInfo->windowId, FALSE);
if (!railWindow)
return TRUE;
@@ -856,7 +856,7 @@ static BOOL xf_rail_window_icon(rdpContext* context, const WINDOW_ORDER_INFO* or
xf_rail_set_window_icon(xfc, railWindow, icon, replaceIcon);
rc = TRUE;
}
xf_rail_return_window(railWindow);
xf_rail_return_window(railWindow, FALSE);
return rc;
}
@@ -868,7 +868,7 @@ static BOOL xf_rail_window_cached_icon(rdpContext* context, const WINDOW_ORDER_I
WINPR_ASSERT(orderInfo);
BOOL replaceIcon = 0;
xfAppWindow* railWindow = xf_rail_get_window(xfc, orderInfo->windowId);
xfAppWindow* railWindow = xf_rail_get_window(xfc, orderInfo->windowId, FALSE);
if (!railWindow)
return TRUE;
@@ -890,7 +890,7 @@ static BOOL xf_rail_window_cached_icon(rdpContext* context, const WINDOW_ORDER_I
xf_rail_set_window_icon(xfc, railWindow, icon, replaceIcon);
rc = TRUE;
}
xf_rail_return_window(railWindow);
xf_rail_return_window(railWindow, FALSE);
return rc;
}
@@ -1123,7 +1123,7 @@ static UINT xf_rail_server_local_move_size(RailClientContext* context,
WINPR_ASSERT(localMoveSize);
xfContext* xfc = (xfContext*)context->custom;
xfAppWindow* appWindow = xf_rail_get_window(xfc, localMoveSize->windowId);
xfAppWindow* appWindow = xf_rail_get_window(xfc, localMoveSize->windowId, FALSE);
if (!appWindow)
return ERROR_INTERNAL_ERROR;
@@ -1208,7 +1208,7 @@ static UINT xf_rail_server_local_move_size(RailClientContext* context,
else
xf_EndLocalMoveSize(xfc, appWindow);
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
return CHANNEL_RC_OK;
}
@@ -1224,7 +1224,7 @@ static UINT xf_rail_server_min_max_info(RailClientContext* context,
WINPR_ASSERT(minMaxInfo);
xfContext* xfc = (xfContext*)context->custom;
xfAppWindow* appWindow = xf_rail_get_window(xfc, minMaxInfo->windowId);
xfAppWindow* appWindow = xf_rail_get_window(xfc, minMaxInfo->windowId, FALSE);
if (appWindow)
{
@@ -1233,7 +1233,7 @@ static UINT xf_rail_server_min_max_info(RailClientContext* context,
minMaxInfo->minTrackHeight, minMaxInfo->maxTrackWidth,
minMaxInfo->maxTrackHeight);
}
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
return CHANNEL_RC_OK;
}
@@ -1401,13 +1401,20 @@ BOOL xf_rail_del_window(xfContext* xfc, UINT64 id)
if (!xfc->railWindows)
return FALSE;
return HashTable_Remove(xfc->railWindows, &id);
xf_lock_x11(xfc);
const BOOL res = HashTable_Remove(xfc->railWindows, &id);
xf_unlock_x11(xfc);
return res;
}
void xf_rail_return_windowFrom(xfAppWindow* window, const char* file, const char* fkt, size_t line)
void xf_rail_return_windowFrom(xfAppWindow* window, BOOL alreadyLocked, const char* file,
const char* fkt, size_t line)
{
if (!window)
return;
if (alreadyLocked)
return;
xfAppWindowsUnlockFrom(window->xfc, file, fkt, line);
}

View File

@@ -125,16 +125,17 @@ BOOL xf_rail_disable_remoteapp_mode(xfContext* xfc);
xfAppWindow* xf_rail_add_window(xfContext* xfc, UINT64 id, INT32 x, INT32 y, UINT32 width,
UINT32 height, UINT32 surfaceId);
#define xf_rail_return_window(window) \
xf_rail_return_windowFrom((window), __FILE__, __func__, __LINE__)
void xf_rail_return_windowFrom(xfAppWindow* window, const char* file, const char* fkt, size_t line);
#define xf_rail_return_window(window, alreadyLocked) \
xf_rail_return_windowFrom((window), (alreadyLocked), __FILE__, __func__, __LINE__)
void xf_rail_return_windowFrom(xfAppWindow* window, BOOL alreadyLocked, const char* file,
const char* fkt, size_t line);
#define xf_rail_get_window(xfc, id) \
xf_rail_get_windowFrom((xfc), (id), __FILE__, __func__, __LINE__)
#define xf_rail_get_window(xfc, id, alreadyLocked) \
xf_rail_get_windowFrom((xfc), (id), (alreadyLocked), __FILE__, __func__, __LINE__)
WINPR_ATTR_MALLOC(xf_rail_return_windowFrom, 1)
xfAppWindow* xf_rail_get_windowFrom(xfContext* xfc, UINT64 id, const char* file, const char* fkt,
size_t line);
xfAppWindow* xf_rail_get_windowFrom(xfContext* xfc, UINT64 id, BOOL alreadyLocked, const char* file,
const char* fkt, size_t line);
BOOL xf_rail_del_window(xfContext* xfc, UINT64 id);

View File

@@ -1371,8 +1371,6 @@ void xf_UpdateWindowArea(xfContext* xfc, xfAppWindow* appWindow, int x, int y, i
if (ay + height > appWindow->windowOffsetY + appWindow->height)
height = (appWindow->windowOffsetY + appWindow->height - 1) - ay;
xf_lock_x11(xfc);
if (freerdp_settings_get_bool(settings, FreeRDP_SoftwareGdi))
{
LogDynAndXPutImage(xfc->log, xfc->display, appWindow->pixmap, appWindow->gc, xfc->image, ax,
@@ -1384,7 +1382,6 @@ void xf_UpdateWindowArea(xfContext* xfc, xfAppWindow* appWindow, int x, int y, i
x, y, WINPR_ASSERTING_INT_CAST(uint32_t, width),
WINPR_ASSERTING_INT_CAST(uint32_t, height), x, y);
LogDynAndXFlush(xfc->log, xfc->display);
xf_unlock_x11(xfc);
}
void xf_AppWindowDestroyImage(xfAppWindow* appWindow)
@@ -1441,8 +1438,8 @@ static xfAppWindow* get_windowUnlocked(xfContext* xfc, UINT64 id)
return HashTable_GetItemValue(xfc->railWindows, &id);
}
xfAppWindow* xf_rail_get_windowFrom(xfContext* xfc, UINT64 id, const char* file, const char* fkt,
size_t line)
xfAppWindow* xf_rail_get_windowFrom(xfContext* xfc, UINT64 id, BOOL alreadyLocked, const char* file,
const char* fkt, size_t line)
{
if (!xfc)
return nullptr;
@@ -1450,9 +1447,12 @@ xfAppWindow* xf_rail_get_windowFrom(xfContext* xfc, UINT64 id, const char* file,
if (!xfc->railWindows)
return nullptr;
xfAppWindowsLockFrom(xfc, file, fkt, line);
if (!alreadyLocked)
xfAppWindowsLockFrom(xfc, file, fkt, line);
xfAppWindow* window = get_windowUnlocked(xfc, id);
if (!window)
if (!window && !alreadyLocked)
xfAppWindowsUnlockFrom(xfc, file, fkt, line);
return window;
@@ -1501,7 +1501,7 @@ UINT xf_AppUpdateWindowFromSurface(xfContext* xfc, gdiGfxSurface* surface)
WINPR_ASSERT(xfc);
WINPR_ASSERT(surface);
xfAppWindow* appWindow = xf_rail_get_window(xfc, surface->windowId);
xfAppWindow* appWindow = xf_rail_get_window(xfc, surface->windowId, FALSE);
if (!appWindow)
{
WLog_VRB(TAG, "Failed to find a window for id=0x%08" PRIx64, surface->windowId);
@@ -1512,7 +1512,6 @@ UINT xf_AppUpdateWindowFromSurface(xfContext* xfc, gdiGfxSurface* surface)
UINT32 nrects = 0;
const RECTANGLE_16* rects = region16_rects(&surface->invalidRegion, &nrects);
xf_lock_x11(xfc);
if (swGdi)
{
if (appWindow->surfaceId != surface->surfaceId)
@@ -1569,9 +1568,9 @@ UINT xf_AppUpdateWindowFromSurface(xfContext* xfc, gdiGfxSurface* surface)
rc = CHANNEL_RC_OK;
fail:
xf_rail_return_window(appWindow);
xf_rail_return_window(appWindow, FALSE);
LogDynAndXFlush(xfc->log, xfc->display);
xf_unlock_x11(xfc);
return rc;
}
@@ -1601,12 +1600,12 @@ void xf_XSetTransientForHint(xfContext* xfc, xfAppWindow* window)
if (window->ownerWindowId == 0)
return;
xfAppWindow* parent = xf_rail_get_window(xfc, window->ownerWindowId);
xfAppWindow* parent = xf_rail_get_window(xfc, window->ownerWindowId, TRUE);
if (!parent)
return;
(void)LogDynAndXSetTransientForHint(xfc->log, xfc->display, window->handle, parent->handle);
xf_rail_return_window(parent);
xf_rail_return_window(parent, TRUE);
}
void xfAppWindowsLockFrom(xfContext* xfc, WINPR_ATTR_UNUSED const char* file,
@@ -1620,6 +1619,7 @@ void xfAppWindowsLockFrom(xfContext* xfc, WINPR_ATTR_UNUSED const char* file,
WLog_PrintTextMessage(xfc->log, level, line, file, fkt, "[rails] locking [%s]", fkt);
#endif
xf_lock_x11(xfc);
HashTable_Lock(xfc->railWindows);
#if defined(WITH_VERBOSE_WINPR_ASSERT)
@@ -1643,4 +1643,5 @@ void xfAppWindowsUnlockFrom(xfContext* xfc, WINPR_ATTR_UNUSED const char* file,
#endif
HashTable_Unlock(xfc->railWindows);
xf_unlock_x11(xfc);
}