[PATCH xserver] animcur: Move timer into pDev->spriteInfo->anim

Aaron Plattner aplattner at nvidia.com
Mon Jan 8 23:02:59 UTC 2018


Commit 094a63d56fbfb9e23210cc9ac538fb198af37cee moved the timer that handles
animated cursors from the per-screen AnimCurScreenRec into the per-cursor
AnimCurRec. However, the timer that runs takes the DeviceIntPtr as its argument,
and looks up which screen the cursor is supposed to be on from that.

This leads to a problem when a device changes from one animated cursor to
another: The timer for the first cursor is not canceled. Then, when the device
transitions to a static cursor, the second timer is canceled
pDev->spriteInfo->anim.pScreen is cleared. Finally, the first timer fires and
AnimCurTimerNotify crashes because pScreen is NULL.

Since there's only ever supposed to be one timer pending for a given device,
move the timer into pDev->spriteInfo->anim.pTimer. This timer is canceled when
transitioning to a static cursor, and re-armed when transitioning from one
animated cursor to another.

Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
Reported-by: https://devtalk.nvidia.com/default/topic/1028172/linux/titan-v-ubuntu-16-04lts-and-387-34-driver-crashes-badly/post/5230967/#5230967
---
I'm not convinced that I fully understood how these timers worked with multiple
X screens before 094a63d56fbf, so hopefully this is an acceptable fix. It seems
to work in my single-X-screen testing.

 dix/devices.c      |  1 +
 include/inputstr.h |  1 +
 render/animcur.c   | 14 ++++----------
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/dix/devices.c b/dix/devices.c
index 4a628afb0f49..98493df58fb6 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -1014,6 +1014,7 @@ CloseDevice(DeviceIntPtr dev)
     free(dev->last.touches);
     dev->config_info = NULL;
     dixFreePrivates(dev->devPrivates, PRIVATE_DEVICE);
+    TimerFree(dev->spriteInfo->anim.pTimer);
     free(dev);
 }
 
diff --git a/include/inputstr.h b/include/inputstr.h
index 5f0026b9b733..7de1ed39874d 100644
--- a/include/inputstr.h
+++ b/include/inputstr.h
@@ -514,6 +514,7 @@ typedef struct _SpriteInfoRec {
     struct {
         CursorPtr pCursor;
         ScreenPtr pScreen;
+        OsTimerPtr pTimer;
         int elt;
     } anim;
 } SpriteInfoRec, *SpriteInfoPtr;
diff --git a/render/animcur.c b/render/animcur.c
index 058bc1b3237c..83cb5572c740 100644
--- a/render/animcur.c
+++ b/render/animcur.c
@@ -55,7 +55,6 @@ typedef struct _AnimCurElt {
 typedef struct _AnimCur {
     int nelt;                   /* number of elements in the elts array */
     AnimCurElt *elts;           /* actually allocated right after the structure */
-    OsTimerPtr timer;
 } AnimCurRec, *AnimCurPtr;
 
 typedef struct _AnimScrPriv {
@@ -171,19 +170,16 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor)
                 pDev->spriteInfo->anim.elt = 0;
                 pDev->spriteInfo->anim.pCursor = pCursor;
                 pDev->spriteInfo->anim.pScreen = pScreen;
-
-                ac->timer = TimerSet(ac->timer, 0, ac->elts[0].delay,
-                                     AnimCurTimerNotify, pDev);
+                pDev->spriteInfo->anim.pTimer =
+                    TimerSet(pDev->spriteInfo->anim.pTimer, 0,
+                             ac->elts[0].delay, AnimCurTimerNotify, pDev);
             }
         }
         else
             ret = TRUE;
     }
     else {
-        CursorPtr old = pDev->spriteInfo->anim.pCursor;
-
-        if (old && IsAnimCur(old))
-            TimerCancel(GetAnimCur(old)->timer);
+        TimerCancel(pDev->spriteInfo->anim.pTimer);
 
         pDev->spriteInfo->anim.pCursor = 0;
         pDev->spriteInfo->anim.pScreen = 0;
@@ -325,13 +321,11 @@ AnimCursorCreate(CursorPtr *cursors, CARD32 *deltas, int ncursor,
     pCursor->id = cid;
 
     ac = GetAnimCur(pCursor);
-    ac->timer = TimerSet(NULL, 0, 0, AnimCurTimerNotify, NULL);
 
     /* security creation/labeling check */
     rc = XaceHook(XACE_RESOURCE_ACCESS, client, cid, RT_CURSOR, pCursor,
                   RT_NONE, NULL, DixCreateAccess);
     if (rc != Success) {
-        TimerFree(ac->timer);
         dixFiniPrivates(pCursor, PRIVATE_CURSOR);
         free(pCursor);
         return rc;
-- 
2.15.1



More information about the xorg-devel mailing list