From 98fad8b1f117c5b48df8a33cc43f62ddf2b6f3e0 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Fri, 20 Sep 2024 13:00:14 +0200 Subject: [PATCH] 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 --- dix/dix_priv.h | 13 ++++++ dix/meson.build | 1 + dix/screen.c | 2 + dix/screen_hooks.c | 37 +++++++++++++++++ dix/window.c | 5 +-- include/dix_screen_hooks.h | 73 ++++++++++++++++++++++++++++++++++ include/meson.build | 2 + include/scrnintstr.h | 15 +++++++ include/xorg-server.h.meson.in | 3 ++ 9 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 dix/screen_hooks.c create mode 100644 include/dix_screen_hooks.h diff --git a/dix/dix_priv.h b/dix/dix_priv.h index 467961e18..76d00df24 100644 --- a/dix/dix_priv.h +++ b/dix/dix_priv.h @@ -257,4 +257,17 @@ void MakePredeclaredAtoms(void); void dixFreeScreen(ScreenPtr pScreen); +/* + * @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 */ diff --git a/dix/meson.build b/dix/meson.build index 658f7f966..190972101 100644 --- a/dix/meson.build +++ b/dix/meson.build @@ -27,6 +27,7 @@ srcs_dix = [ 'region.c', 'registry.c', 'resource.c', + 'screen_hooks.c', 'selection.c', 'screen.c', 'swaprep.c', diff --git a/dix/screen.c b/dix/screen.c index c6fdfa3dc..9bdd92b76 100644 --- a/dix/screen.c +++ b/dix/screen.c @@ -4,6 +4,7 @@ */ #include +#include "dix/callback_priv.h" #include "dix/dix_priv.h" #include "dix/gc_priv.h" #include "include/screenint.h" @@ -19,5 +20,6 @@ void dixFreeScreen(ScreenPtr pScreen) dixFreeScreenSpecificPrivates(pScreen); pScreen->CloseScreen(pScreen); dixFreePrivates(pScreen->devPrivates, PRIVATE_SCREEN); + DeleteCallbackList(&pScreen->hookWindowDestroy); free(pScreen); } diff --git a/dix/screen_hooks.c b/dix/screen_hooks.c new file mode 100644 index 000000000..a273b1be4 --- /dev/null +++ b/dix/screen_hooks.c @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: MIT OR X11 + * + * Copyright © 2024 Enrico Weigelt, metux IT consult + */ + +#include + +#include "dix/dix_priv.h" +#include "include/dix.h" +#include "include/os.h" +#include "include/scrnintstr.h" +#include "include/windowstr.h" + +#define DECLARE_HOOK_PROC(NAME, FIELD, TYPE) \ + void dixScreenHook##NAME(ScreenPtr pScreen, TYPE func) \ + { \ + AddCallback(&pScreen->FIELD, (CallbackProcPtr)func, pScreen); \ + } \ + \ + void dixScreenUnhook##NAME(ScreenPtr pScreen, TYPE func) \ + { \ + DeleteCallback(&pScreen->FIELD, (CallbackProcPtr)func, pScreen); \ + } + +DECLARE_HOOK_PROC(WindowDestroy, hookWindowDestroy, XorgScreenWindowDestroyProcPtr); + +int dixScreenRaiseWindowDestroy(WindowPtr pWin) +{ + if (!pWin) + return Success; + + ScreenPtr pScreen = pWin->drawable.pScreen; + + CallCallbacks(&pScreen->hookWindowDestroy, pWin); + + return (pScreen->DestroyWindow ? pScreen->DestroyWindow(pWin) : Success); +} diff --git a/dix/window.c b/dix/window.c index 672593512..901fcfc77 100644 --- a/dix/window.c +++ b/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); } diff --git a/include/dix_screen_hooks.h b/include/dix_screen_hooks.h new file mode 100644 index 000000000..db2260506 --- /dev/null +++ b/include/dix_screen_hooks.h @@ -0,0 +1,73 @@ +/* SPDX-License-Identifier: MIT OR X11 + * + * Copyright © 2024 Enrico Weigelt, metux IT consult + * + * @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 + +#include "screenint.h" /* ScreenPtr */ +#include "window.h" /* WindowPtr */ + +/* prototype of a window destructor */ +typedef void (*XorgScreenWindowDestroyProcPtr)(CallbackListPtr *pcbl, + ScreenPtr pScreen, + WindowPtr pWindow); + +/** + * @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, + XorgScreenWindowDestroyProcPtr func); + +/** + * @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, + XorgScreenWindowDestroyProcPtr func); + +#endif /* DIX_SCREEN_HOOKS_H */ diff --git a/include/meson.build b/include/meson.build index 994b4cfba..6b5953f7d 100644 --- a/include/meson.build +++ b/include/meson.build @@ -444,6 +444,7 @@ if with_dtrace dtrace_hdr += dtrace_header.process(dtrace_tmpl) endif +# install SDK headers if build_xorg install_data( [ @@ -457,6 +458,7 @@ if build_xorg 'cursor.h', 'cursorstr.h', 'dix.h', + 'dix_screen_hooks.h', 'dixaccess.h', 'dixevents.h', 'dixfont.h', diff --git a/include/scrnintstr.h b/include/scrnintstr.h index 98f47bf51..155ad57ca 100644 --- a/include/scrnintstr.h +++ b/include/scrnintstr.h @@ -57,6 +57,8 @@ SOFTWARE. #include "privates.h" #include +#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; @@ -658,6 +669,10 @@ typedef struct _Screen { ReplaceScanoutPixmapProcPtr ReplaceScanoutPixmap; XYToWindowProcPtr XYToWindow; DPMSProcPtr DPMS; + + /* additional window destructors (replaces wrapping DestroyWindow). + should NOT be touched outside of DIX core */ + CallbackListPtr hookWindowDestroy; } ScreenRec; static inline RegionPtr diff --git a/include/xorg-server.h.meson.in b/include/xorg-server.h.meson.in index d0572c9d4..392d8ede5 100644 --- a/include/xorg-server.h.meson.in +++ b/include/xorg-server.h.meson.in @@ -214,4 +214,7 @@ /* byte order */ #mesondefine X_BYTE_ORDER +/* announce server API features */ +#define XORG_API_DIX_SCREEN_HOOK_WINDOW_DESTROY 1 + #endif /* _XORG_SERVER_H_ */