(!1714) dix: add per-screen window destructor hook

Right now, extension specific window destruction procedures are implemented
by wrapping the ScreenRec's DestroyWindow() proc pointer: the extensions are
storing the original pointer in their private data and putting in their own one.
On each call, their proc restores the original one, calls it, and switches back
again. When multiple extensions doing so, they're forming a kind of daisy chain.
(the same is done for lots of other procs)

While that approach is looking nice and elegant on the drawing board, it's
complicated, dangerous like a chainsaw and makes debugging hard, leading to
pretty blurred API borders.

This commit introduces a simple approach for letting extension hook into the
window destruction safely, w/o having to care much about side effects with
the call chain. Extensions now can simply register their destructor proc
(and an opaque pointer) and get called back - w/o ever having to mess with
the ScreenRec's internal structures.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

dix/dix_priv.h

@@ -253,4 +253,17 @@ extern Bool explicit_display;
 extern Bool disableBackingStore;
 extern Bool enableBackingStore;
 
+/*
+ * @brief call screen's window destructors
+ * @see dixScreenHookWindowDestroy
+ * @param pWin the window thats being destroyed
+ * @result the ScreenRec's DestroyWindow() return value
+ *
+ * Call the pluggable window destructors that extensions might have registered on
+ * the screen, and finally call ScreenRec's DestroyWindow proc.
+ *
+ * Should only be called by DIX itself.
+ */
+int dixScreenRaiseWindowDestroy(WindowPtr pWin);
+
 #endif /* _XSERVER_DIX_PRIV_H */

dix/meson.build

@@ -27,6 +27,7 @@ srcs_dix = [
     'region.c',
     'registry.c',
     'resource.c',
+    'screen_hooks.c',
     'selection.c',
     'swaprep.c',
     'swapreq.c',

dix/screen_hooks.c

@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: MIT OR X11
+ *
+ * Copyright © 2024 Enrico Weigelt, metux IT consult <info@metux.net>
+ */
+
+#include <dix-config.h>
+
+#include "dix/dix_priv.h"
+#include "include/dix.h"
+#include "include/os.h"
+#include "include/scrnintstr.h"
+#include "include/windowstr.h"
+
+#define ARRAY_LENGTH(x) (sizeof(x) / sizeof((x)[0]))
+
+#define ARRAY_FOR_EACH(_ARRAY, _WALK) \
+    for (struct { int idx; typeof(_ARRAY[0])*ptr; } _WALK = { 0, _ARRAY }; _WALK.idx < ARRAY_LENGTH(_ARRAY); _WALK.idx++, _WALK.ptr++)
+
+#define DECLARE_HOOK_LIST(NAME, FIELD) \
+    void dixScreenHook##NAME(ScreenPtr pScreen, typeof(((ScreenRec){0}).FIELD[0].func) func, void *arg) \
+    { \
+        for (int i=0; i<ARRAY_LENGTH(pScreen->FIELD); i++) { \
+            if (!(pScreen->FIELD[i].func)) { \
+                pScreen->FIELD[i].func = func; \
+                pScreen->FIELD[i].arg = arg; \
+                return; \
+            } \
+        } \
+        FatalError("%s: out of slots", __FUNCTION__); \
+    } \
+    \
+    void dixScreenUnhook##NAME(ScreenPtr pScreen, typeof(((ScreenRec){0}).FIELD[0].func) func, void *arg) \
+    { \
+        for (int i=0; i<ARRAY_LENGTH(pScreen->FIELD); i++) { \
+            if ((pScreen->FIELD[i].func == func) && (pScreen->FIELD[i].arg == arg)) { \
+                pScreen->FIELD[i].func = NULL; \
+                pScreen->FIELD[i].arg = NULL; \
+                return; \
+            } \
+        } \
+    }
+
+DECLARE_HOOK_LIST(WindowDestroy, _notify_window_destroy)
+
+int dixScreenRaiseWindowDestroy(WindowPtr pWin)
+{
+    if (!pWin)
+        return Success;
+
+    ScreenPtr pScreen = pWin->drawable.pScreen;
+
+    ARRAY_FOR_EACH(pScreen->_notify_window_destroy, walk) {
+        if (walk.ptr->func)
+            walk.ptr->func(pScreen, pWin, walk.ptr->arg);
+    }
+    return (pScreen->DestroyWindow ? (*pScreen->DestroyWindow) (pWin) : Success);
+}

dix/window.c

@@ -998,8 +998,6 @@ DisposeWindowOptional(WindowPtr pWin)
 static void
 FreeWindowResources(WindowPtr pWin)
 {
-    ScreenPtr pScreen = pWin->drawable.pScreen;
-
     DeleteWindowFromAnySaveSet(pWin);
     DeleteWindowFromAnySelections(pWin);
     DeleteWindowFromAnyEvents(pWin, TRUE);
@@ -1019,8 +1017,9 @@ FreeWindowResources(WindowPtr pWin)
         dixDestroyPixmap(pWin->background.pixmap, 0);
 
     DeleteAllWindowProperties(pWin);
+
     /* We SHOULD check for an error value here XXX */
-    (*pScreen->DestroyWindow) (pWin);
+    dixScreenRaiseWindowDestroy(pWin);
     DisposeWindowOptional(pWin);
 }
 

include/dix_screen_hooks.h

@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: MIT OR X11
+ *
+ * Copyright © 2024 Enrico Weigelt, metux IT consult <info@metux.net>
+ *
+ * @brief exported API entry points for hooking into screen operations
+ *
+ * These hooks are replacing the old, complicated approach of wrapping
+ * ScreenRec's proc vectors. Unlike the wrapping, these hooks are designed
+ * to be safe against changes in setup/teardown order and are called
+ * independently of the ScreenProc call vectors. It is guaranteed that the
+ * objects to operate on already/still exist (eg. destructors are callled
+ * before the object is actually destroyed, while post-create hooks are
+ * called after the object is created)
+ *
+ * Main consumers are extensions that need to associate extra data or
+ * doing other things additional to the original operation. In some cases
+ * they might even be used in drivers (in order to split device specific
+ * from generic logic)
+ */
+#ifndef DIX_SCREEN_HOOKS_H
+#define DIX_SCREEN_HOOKS_H
+
+#include <X11/Xfuncproto.h>
+
+#include "screenint.h" /* ScreenPtr */
+#include "window.h" /* WindowPtr */
+
+/* prototype of a window destructor */
+typedef void (*XorgWindowDestroyProcPtr)(ScreenPtr pScreen,
+                                         WindowPtr pWindow,
+                                         void *arg);
+
+/**
+ * @brief register a window on the given screen
+ *
+ * @param pScreen pointer to the screen to register the destructor into
+ * @param func pointer to the window destructor function
+ * @param arg opaque pointer passed to the destructor
+ *
+ * Window destructors are the replacement for fragile and complicated wrapping of
+ * pScreen->DestroyWindow(): extensions can safely register there custom destructors
+ * here, without ever caring about anybody else.
+ +
+ * The destructors are run right before pScreen->DestroyWindow() - when the window
+ * is already removed from hierarchy (thus cannot receive any events anymore) and
+ * most of it's data already destroyed - and supposed to do necessary per-extension
+ * cleanup duties. Their execution order is *unspecified*.
+ *
+ * Screen drivers (DDX'es, xf86 video drivers, ...) shall not use these, but still
+ * set the pScreen->DestroyWindow pointer - and these should be the *only* ones
+ * ever setting it.
+ *
+ * When registration fails, the server aborts.
+ *
+ **/
+_X_EXPORT void dixScreenHookWindowDestroy(ScreenPtr pScreen,
+                                          XorgWindowDestroyProcPtr func,
+                                          void *arg);
+
+/**
+ * @brief unregister a window destructor on the given screen
+ *
+ * @param pScreen pointer to the screen to unregister the destructor from
+ * @param func pointer to the window destructor function
+ * @param arg opaque pointer passed to the destructor
+ *
+ * @see dixScreenHookWindowDestroy
+ *
+ * Unregister a window destructor hook registered via @ref dixScreenHookWindowDestroy
+ **/
+_X_EXPORT void dixScreenUnhookWindowDestroy(ScreenPtr pScreen,
+                                            XorgWindowDestroyProcPtr func,
+                                            void *arg);
+
+#endif /* DIX_SCREEN_HOOKS_H */

include/meson.build

@@ -438,6 +438,7 @@ if with_dtrace
     dtrace_hdr += dtrace_header.process(dtrace_tmpl)
 endif
 
+# install SDK headers
 if build_xorg
     install_data(
         [
@@ -451,6 +452,7 @@ if build_xorg
             'cursor.h',
             'cursorstr.h',
             'dix.h',
+            'dix_screen_hooks.h',
             'dixaccess.h',
             'dixevents.h',
             'dixfont.h',

include/scrnintstr.h

@@ -57,6 +57,8 @@ SOFTWARE.
 #include "privates.h"
 #include <X11/extensions/randr.h>
 
+#include "dix_screen_hooks.h"
+
 typedef struct _PixmapFormat {
     unsigned char depth;
     unsigned char bitsPerPixel;
@@ -490,8 +492,17 @@ typedef void (*DPMSProcPtr)(ScreenPtr pScreen, int level);
     required. Unwrap occurs at the top of each function, just after
     entry, and Wrap occurs at the bottom of each function, just
     before returning.
+
+    DestroyWindow() should NOT be wrapped anymore
+    use dixScreenHookWindowDestroy() instead.
  */
 
+#define _SCREEN_HOOK_TYPE(NAME, FUNCTYPE, ARRSIZE) \
+    struct { \
+        FUNCTYPE func; \
+        void *arg; \
+    } NAME[ARRSIZE];
+
 typedef struct _Screen {
     int myNum;                  /* index of this instance in Screens[] */
     ATOM id;
@@ -546,6 +557,10 @@ typedef struct _Screen {
     RestackWindowProcPtr RestackWindow;
     PaintWindowProcPtr PaintWindow;
 
+    /* additional window destructors (replaces wrapping DestroyWindow).
+       should NOT be touched outside of DIX core */
+    _SCREEN_HOOK_TYPE(_notify_window_destroy, XorgWindowDestroyProcPtr, 8);
+
     /* Pixmap procedures */
 
     CreatePixmapProcPtr CreatePixmap;

include/xorg-server.h.meson.in

@@ -216,4 +216,7 @@
 /* byte order */
 #mesondefine X_BYTE_ORDER
 
+/* announce server API features */
+#define XORG_API_DIX_SCREEN_HOOK_WINDOW_DESTROY 1
+
 #endif /* _XORG_SERVER_H_ */