Fix XTS test XCopyColormapAndFree/5 hangs

Upstream merge request:
https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/254

Resolves: https://issues.redhat.com/browse/RHEL-40132
This commit is contained in:
José Expósito 2024-06-20 10:30:33 +02:00
parent fa7172b7ab
commit cfbe5d4947
4 changed files with 224 additions and 1 deletions

View File

@ -0,0 +1,34 @@
From 5dfedaf4aa1a032ea6cb4e871abd2e065f798129 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Thu, 6 Jun 2024 16:25:26 +0200
Subject: [PATCH 1/3] Revert "Fix XTS regression in XCopyColormapAndFree"
This change was to fix the next change that we are to revert as well.
This reverts commit 68c72a7341b114277ab232f2499ee3bd035af8a0.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/254>
---
src/CopyCmap.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/CopyCmap.c b/src/CopyCmap.c
index b37aba73..b4954b01 100644
--- a/src/CopyCmap.c
+++ b/src/CopyCmap.c
@@ -53,11 +53,6 @@ Colormap XCopyColormapAndFree(
mid = req->mid = XAllocID(dpy);
req->srcCmap = src_cmap;
- /* re-lock the display to keep XID handling in sync */
- UnlockDisplay(dpy);
- SyncHandle();
- LockDisplay(dpy);
-
#if XCMS
_XcmsCopyCmapRecAndFree(dpy, src_cmap, mid);
#endif
--
2.45.2

View File

@ -0,0 +1,92 @@
From 739fce4c12c7aa39112353d80c8a3bf25bdd5274 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 7 Jun 2024 09:07:39 +0200
Subject: [PATCH 2/3] Revert "Protect colormap add/removal with display lock"
That commit 99a2cf1aa was moving the calls to the _Xcms*CmapRec*()
family of functions within a display lock to make the XCMS colormap
functions thread safe.
Unfortunately, that causes a deadlock in XCopyColormapAndFree(), because
_XcmsCopyCmapRecAndFree() calls CmapRecForColormap() which calls
XGetVisualInfo() which also tries to acquire the display lock.
So, instead of moving the entire functions within the display lock,
let's try to make the functions themselves thread safe in the following
commit, and revert this change which causes a deadlock.
This reverts commit 99a2cf1aa0b58391078d5d3edf0a7dab18c7745d.
Fixes: https://gitlab.freedesktop.org/xorg/lib/libx11/-/issues/215
See-also: https://gitlab.freedesktop.org/xorg/lib/libx11/-/issues/94
Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/254>
---
src/CopyCmap.c | 6 +++---
src/CrCmap.c | 6 +++---
src/FreeCmap.c | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/CopyCmap.c b/src/CopyCmap.c
index b4954b01..5444550c 100644
--- a/src/CopyCmap.c
+++ b/src/CopyCmap.c
@@ -53,12 +53,12 @@ Colormap XCopyColormapAndFree(
mid = req->mid = XAllocID(dpy);
req->srcCmap = src_cmap;
+ UnlockDisplay(dpy);
+ SyncHandle();
+
#if XCMS
_XcmsCopyCmapRecAndFree(dpy, src_cmap, mid);
#endif
- UnlockDisplay(dpy);
- SyncHandle();
-
return(mid);
}
diff --git a/src/CrCmap.c b/src/CrCmap.c
index 1b18a15b..9904c7dd 100644
--- a/src/CrCmap.c
+++ b/src/CrCmap.c
@@ -48,12 +48,12 @@ Colormap XCreateColormap(
if (visual == CopyFromParent) req->visual = CopyFromParent;
else req->visual = visual->visualid;
+ UnlockDisplay(dpy);
+ SyncHandle();
+
#ifdef XCMS
_XcmsAddCmapRec(dpy, mid, w, visual);
#endif
- UnlockDisplay(dpy);
- SyncHandle();
-
return(mid);
}
diff --git a/src/FreeCmap.c b/src/FreeCmap.c
index 68496dd8..e2b76fa6 100644
--- a/src/FreeCmap.c
+++ b/src/FreeCmap.c
@@ -41,12 +41,12 @@ XFreeColormap(
LockDisplay(dpy);
GetResReq(FreeColormap, cmap, req);
+ UnlockDisplay(dpy);
+ SyncHandle();
+
#ifdef XCMS
_XcmsDeleteCmapRec(dpy, cmap);
#endif
- UnlockDisplay(dpy);
- SyncHandle();
-
return 1;
}
--
2.45.2

View File

@ -0,0 +1,92 @@
From 1472048b7a02d1b7fc25cfeda761db23fba21eac Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 7 Jun 2024 09:05:55 +0200
Subject: [PATCH 3/3] Make colormap private interfaces thread safe.
Protect access to the dpy structure by a display lock, so that these can
be called outside of a global display lock.
That allows the XCMS colormap functions to be thread safe without having
the whole functions within a display lock, to avoid deadlocks.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
See-also: https://gitlab.freedesktop.org/xorg/lib/libx11/-/issues/215
See-also: https://gitlab.freedesktop.org/xorg/lib/libx11/-/issues/94
Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/254>
---
src/xcms/cmsCmap.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/xcms/cmsCmap.c b/src/xcms/cmsCmap.c
index c7087ecb..4b229477 100644
--- a/src/xcms/cmsCmap.c
+++ b/src/xcms/cmsCmap.c
@@ -87,12 +87,17 @@ CmapRecForColormap(
_XAsyncHandler async;
_XAsyncErrorState async_state;
+ LockDisplay(dpy);
for (pRec = (XcmsCmapRec *)dpy->cms.clientCmaps; pRec != NULL;
pRec = pRec->pNext) {
if (pRec->cmapID == cmap) {
+ UnlockDisplay(dpy);
+ SyncHandle();
return(pRec);
}
}
+ UnlockDisplay(dpy);
+ SyncHandle();
/*
* Can't find an XcmsCmapRec associated with cmap in our records.
@@ -258,9 +263,12 @@ _XcmsAddCmapRec(
pNew->dpy = dpy;
pNew->windowID = windowID;
pNew->visual = visual;
+ LockDisplay(dpy);
pNew->pNext = (XcmsCmapRec *)dpy->cms.clientCmaps;
dpy->cms.clientCmaps = (XPointer)pNew;
dpy->free_funcs->clientCmaps = _XcmsFreeClientCmaps;
+ UnlockDisplay(dpy);
+ SyncHandle();
/*
* Note, we don't create the XcmsCCC for pNew->ccc here because
@@ -342,6 +350,7 @@ _XcmsDeleteCmapRec(
}
/* search for it in the list */
+ LockDisplay(dpy);
pPrevPtr = (XcmsCmapRec **)&dpy->cms.clientCmaps;
while ((pRec = *pPrevPtr) && (pRec->cmapID != cmap)) {
pPrevPtr = &pRec->pNext;
@@ -354,6 +363,8 @@ _XcmsDeleteCmapRec(
*pPrevPtr = pRec->pNext;
Xfree(pRec);
}
+ UnlockDisplay(dpy);
+ SyncHandle();
}
@@ -378,6 +389,7 @@ _XcmsFreeClientCmaps(
{
XcmsCmapRec *pRecNext, *pRecFree;
+ LockDisplay(dpy);
pRecNext = (XcmsCmapRec *)dpy->cms.clientCmaps;
while (pRecNext != NULL) {
pRecFree = pRecNext;
@@ -390,6 +402,8 @@ _XcmsFreeClientCmaps(
Xfree(pRecFree);
}
dpy->cms.clientCmaps = (XPointer)NULL;
+ UnlockDisplay(dpy);
+ SyncHandle();
}
--
2.45.2

View File

@ -18,7 +18,12 @@ Source0: https://xorg.freedesktop.org/archive/individual/lib/%{name}-%{version}.
%endif
Patch2: dont-forward-keycode-0.patch
Patch02: dont-forward-keycode-0.patch
# https://issues.redhat.com/browse/RHEL-40132
Patch03: 0001-Revert-Fix-XTS-regression-in-XCopyColormapAndFree.patch
Patch04: 0002-Revert-Protect-colormap-add-removal-with-display-loc.patch
Patch05: 0003-Make-colormap-private-interfaces-thread-safe.patch
BuildRequires: libtool
BuildRequires: make