From efa65a0317e12c9ad34fa00fe90bf5eae9fa2670 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 4 May 2008 21:52:58 -0700 Subject: Rework composite overlay window code to fix several resource management bugs. The composite overlay window code had several misunderstandings of the workings of the X server, in particular error handling paths would often double-free objects. Clean all of this up by using resource destruction as the sole mechanism for freeing resource-based objects. --- composite/Makefile.am | 1 + composite/compext.c | 193 ++++++------------------------------------------ composite/compinit.c | 13 +--- composite/compint.h | 24 +++++- composite/compoverlay.c | 159 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 206 insertions(+), 184 deletions(-) create mode 100644 composite/compoverlay.c (limited to 'composite') diff --git a/composite/Makefile.am b/composite/Makefile.am index 21504e659..d7bead137 100644 --- a/composite/Makefile.am +++ b/composite/Makefile.am @@ -7,4 +7,5 @@ libcomposite_la_SOURCES = \ compext.c \ compint.h \ compinit.c \ + compoverlay.c \ compwindow.c diff --git a/composite/compext.c b/composite/compext.c index b3433f72b..e720f6ce7 100644 --- a/composite/compext.c +++ b/composite/compext.c @@ -54,10 +54,7 @@ static CARD8 CompositeReqCode; static DevPrivateKey CompositeClientPrivateKey = &CompositeClientPrivateKey; RESTYPE CompositeClientWindowType; RESTYPE CompositeClientSubwindowsType; -static RESTYPE CompositeClientOverlayType; - -static void deleteCompOverlayClient (CompOverlayClientPtr pOcToDel, - ScreenPtr pScreen); +RESTYPE CompositeClientOverlayType; typedef struct _CompositeClient { int major_version; @@ -107,19 +104,8 @@ static int FreeCompositeClientOverlay (pointer value, XID ccwid) { CompOverlayClientPtr pOc = (CompOverlayClientPtr) value; - ScreenPtr pScreen = pOc->pScreen; - CompScreenPtr cs; - - deleteCompOverlayClient(pOc, pScreen); - - /* Unmap overlay window when there are no more clients using it */ - cs = GetCompScreen(pScreen); - if (cs->pOverlayClients == NULL) { - if (cs->pOverlayWin != NULL) { - UnmapWindow(cs->pOverlayWin, FALSE); - } - } + compFreeOverlayClient (pOc); return Success; } @@ -304,137 +290,6 @@ ProcCompositeNameWindowPixmap (ClientPtr client) } -/* - * Routines for manipulating the per-screen overlay clients list. - * This list indicates which clients have called GetOverlayWindow - * for this screen. - */ - -/* Return the screen's overlay client list element for the given client */ -static CompOverlayClientPtr -findCompOverlayClient (ClientPtr pClient, ScreenPtr pScreen) -{ - CompScreenPtr cs = GetCompScreen(pScreen); - CompOverlayClientPtr pOc; - - for (pOc = cs->pOverlayClients; pOc != NULL; pOc = pOc->pNext) { - if (pOc->pClient == pClient) { - return pOc; - } - } - - return NULL; -} - -static int -createCompOverlayClient (ClientPtr pClient, ScreenPtr pScreen) -{ - CompScreenPtr cs = GetCompScreen(pScreen); - CompOverlayClientPtr pOc; - - pOc = (CompOverlayClientPtr) xalloc(sizeof(CompOverlayClientRec)); - if (pOc == NULL) { - return BadAlloc; - } - pOc->pClient = pClient; - pOc->pScreen = pScreen; - pOc->resource = FakeClientID(pClient->index); - pOc->pNext = cs->pOverlayClients; - cs->pOverlayClients = pOc; - - /* - * Create a resource for this element so it can be deleted - * when the client goes away. - */ - if (!AddResource (pOc->resource, CompositeClientOverlayType, - (pointer) pOc)) { - xfree(pOc); - return BadAlloc; - } - - return Success; -} - -/* - * Delete the given overlay client list element from its screen list. - */ -static void -deleteCompOverlayClient (CompOverlayClientPtr pOcToDel, ScreenPtr pScreen) -{ - CompScreenPtr cs = GetCompScreen(pScreen); - CompOverlayClientPtr pOc, pNext; - CompOverlayClientPtr pOcLast = NULL; - - pOc = cs->pOverlayClients; - while (pOc != NULL) { - pNext = pOc->pNext; - if (pOc == pOcToDel) { - xfree(pOc); - if (pOcLast == NULL) { - cs->pOverlayClients = pNext; - } else { - pOcLast->pNext = pNext; - } - break; - } - pOcLast = pOc; - pOc = pNext; - } -} - -/* - * Delete all the hide-counts list elements for this screen. - */ -void -deleteCompOverlayClientsForScreen (ScreenPtr pScreen) -{ - CompScreenPtr cs = GetCompScreen(pScreen); - CompOverlayClientPtr pOc, pTmp; - - pOc = cs->pOverlayClients; - while (pOc != NULL) { - pTmp = pOc->pNext; - FreeResource(pOc->resource, 0); - pOc = pTmp; - } - cs->pOverlayClients = NULL; -} - -/* -** If necessary, create the overlay window. And map it -** Note: I found it excessively difficult to destroy this window -** during compCloseScreen; DeleteWindow can't be called because -** the input devices are already shut down. So we are going to -** just allocate an overlay window once per screen per X server -** invocation. -*/ - -static WindowPtr -createOverlayWindow (ScreenPtr pScreen) -{ - int wid = FakeClientID(0); - WindowPtr pWin; - XID overrideRedirect = TRUE; - int result; - - pWin = CreateWindow ( - wid, WindowTable[pScreen->myNum], - 0, 0, pScreen->width, pScreen->height, 0, - InputOutput, CWOverrideRedirect, &overrideRedirect, - WindowTable[pScreen->myNum]->drawable.depth, - serverClient, pScreen->rootVisual, &result); - if (pWin == NULL) { - return NULL; - } - - if (!AddResource(wid, RT_WINDOW, (pointer)pWin)) { - DeleteWindow(pWin, None); - return NULL; - } - - return pWin; -} - static int ProcCompositeGetOverlayWindow (ClientPtr client) { @@ -456,28 +311,31 @@ ProcCompositeGetOverlayWindow (ClientPtr client) } pScreen = pWin->drawable.pScreen; + /* + * Create an OverlayClient structure to mark this client's + * interest in the overlay window + */ + pOc = compCreateOverlayClient(pScreen, client); + if (pOc == NULL) + return BadAlloc; + + /* + * Make sure the overlay window exists + */ cs = GetCompScreen(pScreen); - if (cs->pOverlayWin == NULL) { - cs->pOverlayWin = createOverlayWindow(pScreen); - if (cs->pOverlayWin == NULL) { + if (cs->pOverlayWin == NULL) + if (!compCreateOverlayWindow(pScreen)) + { + FreeResource (pOc->resource, RT_NONE); return BadAlloc; } - } rc = XaceHook(XACE_RESOURCE_ACCESS, client, cs->pOverlayWin->drawable.id, RT_WINDOW, cs->pOverlayWin, RT_NONE, NULL, DixGetAttrAccess); if (rc != Success) + { + FreeResource (pOc->resource, RT_NONE); return rc; - - MapWindow(cs->pOverlayWin, serverClient); - - /* Record that client is using this overlay window */ - pOc = findCompOverlayClient(client, pScreen); - if (pOc == NULL) { - int ret = createCompOverlayClient(client, pScreen); - if (ret != Success) { - return ret; - } } rep.type = X_Reply; @@ -504,7 +362,6 @@ ProcCompositeReleaseOverlayWindow (ClientPtr client) WindowPtr pWin; ScreenPtr pScreen; CompOverlayClientPtr pOc; - CompScreenPtr cs; REQUEST_SIZE_MATCH(xCompositeReleaseOverlayWindowReq); pWin = (WindowPtr) LookupIDByType (stuff->window, RT_WINDOW); @@ -519,18 +376,12 @@ ProcCompositeReleaseOverlayWindow (ClientPtr client) * Has client queried a reference to the overlay window * on this screen? If not, generate an error. */ - pOc = findCompOverlayClient(client, pWin->drawable.pScreen); - if (pOc == NULL) { + pOc = compFindOverlayClient (pWin->drawable.pScreen, client); + if (pOc == NULL) return BadMatch; - } /* The delete function will free the client structure */ - FreeResource (pOc->resource, 0); - - cs = GetCompScreen(pScreen); - if (cs->pOverlayClients == NULL) { - UnmapWindow(cs->pOverlayWin, FALSE); - } + FreeResource (pOc->resource, RT_NONE); return client->noClientException; } diff --git a/composite/compinit.c b/composite/compinit.c index 49b2044b0..7914a8d25 100644 --- a/composite/compinit.c +++ b/composite/compinit.c @@ -76,14 +76,6 @@ compCloseScreen (int index, ScreenPtr pScreen) pScreen->CopyWindow = cs->CopyWindow; pScreen->PositionWindow = cs->PositionWindow; - deleteCompOverlayClientsForScreen(pScreen); - - /* - ** Note: no need to call DeleteWindow; the server has - ** already destroyed it. - */ - cs->pOverlayWin = NULL; - xfree (cs); dixSetPrivate(&pScreen->devPrivates, CompScreenPrivateKey, NULL); ret = (*pScreen->CloseScreen) (index, pScreen); @@ -122,11 +114,11 @@ compChangeWindowAttributes(WindowPtr pWin, unsigned long mask) if (ret && (mask & CWBackingStore)) { if (pWin->backingStore != NotUseful) { compRedirectWindow(serverClient, pWin, CompositeRedirectAutomatic); - pWin->backStorage = TRUE; + pWin->backStorage = (pointer) (intptr_t) 1; } else { compUnredirectWindow(serverClient, pWin, CompositeRedirectAutomatic); - pWin->backStorage = FALSE; + pWin->backStorage = NULL; } } @@ -380,6 +372,7 @@ compScreenInit (ScreenPtr pScreen) return FALSE; cs->damaged = FALSE; + cs->overlayWid = FakeClientID(0); cs->pOverlayWin = NULL; cs->pOverlayClients = NULL; diff --git a/composite/compint.h b/composite/compint.h index 4b0fe0834..1c19ccd39 100644 --- a/composite/compint.h +++ b/composite/compint.h @@ -155,6 +155,7 @@ typedef struct _CompScreen { VisualID *alternateVisuals; WindowPtr pOverlayWin; + Window overlayWid; CompOverlayClientPtr pOverlayClients; } CompScreenRec, *CompScreenPtr; @@ -172,6 +173,7 @@ extern DevPrivateKey CompSubwindowsPrivateKey; extern RESTYPE CompositeClientWindowType; extern RESTYPE CompositeClientSubwindowsType; +extern RESTYPE CompositeClientOverlayType; /* * compalloc.c @@ -229,6 +231,25 @@ CompositeRegisterAlternateVisuals (ScreenPtr pScreen, Bool compScreenInit (ScreenPtr pScreen); +/* + * compoverlay.c + */ + +void +compFreeOverlayClient (CompOverlayClientPtr pOcToDel); + +CompOverlayClientPtr +compFindOverlayClient (ScreenPtr pScreen, ClientPtr pClient); + +CompOverlayClientPtr +compCreateOverlayClient (ScreenPtr pScreen, ClientPtr pClient); + +Bool +compCreateOverlayWindow (ScreenPtr pScreen); + +void +compDestroyOverlayWindow (ScreenPtr pScreen); + /* * compwindow.c */ @@ -292,9 +313,6 @@ compCopyWindow (WindowPtr pWin, DDXPointRec ptOldOrg, RegionPtr prgnSrc); void compWindowUpdate (WindowPtr pWin); -void -deleteCompOverlayClientsForScreen (ScreenPtr pScreen); - WindowPtr CompositeRealChildHead (WindowPtr pWin); diff --git a/composite/compoverlay.c b/composite/compoverlay.c new file mode 100644 index 000000000..94e5b0346 --- /dev/null +++ b/composite/compoverlay.c @@ -0,0 +1,159 @@ +/* + * Copyright © 2006 Sun Microsystems + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of Sun Microsystems not be used in + * advertising or publicity pertaining to distribution of the software without + * specific, written prior permission. Sun Microsystems makes no + * representations about the suitability of this software for any purpose. It + * is provided "as is" without express or implied warranty. + * + * SUN MICROSYSTEMS DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL SUN MICROSYSTEMS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR + * PERFORMANCE OF THIS SOFTWARE. + * + * Copyright © 2003 Keith Packard + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of Keith Packard not be used in + * advertising or publicity pertaining to distribution of the software without + * specific, written prior permission. Keith Packard makes no + * representations about the suitability of this software for any purpose. It + * is provided "as is" without express or implied warranty. + * + * KEITH PACKARD DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL KEITH PACKARD BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR + * PERFORMANCE OF THIS SOFTWARE. + */ + +#ifdef HAVE_DIX_CONFIG_H +#include +#endif + +#include "compint.h" +#include "xace.h" + +/* + * Delete the given overlay client list element from its screen list. + */ +void +compFreeOverlayClient (CompOverlayClientPtr pOcToDel) +{ + ScreenPtr pScreen = pOcToDel->pScreen; + CompScreenPtr cs = GetCompScreen (pScreen); + CompOverlayClientPtr *pPrev, pOc; + + for (pPrev = &cs->pOverlayClients; (pOc = *pPrev); pPrev = &pOc->pNext) + { + if (pOc == pOcToDel) { + *pPrev = pOc->pNext; + xfree (pOc); + break; + } + } + + /* Destroy overlay window when there are no more clients using it */ + if (cs->pOverlayClients == NULL) + compDestroyOverlayWindow (pScreen); +} + +/* + * Return the client's first overlay client rec from the given screen + */ +CompOverlayClientPtr +compFindOverlayClient (ScreenPtr pScreen, ClientPtr pClient) +{ + CompScreenPtr cs = GetCompScreen(pScreen); + CompOverlayClientPtr pOc; + + for (pOc = cs->pOverlayClients; pOc != NULL; pOc = pOc->pNext) + if (pOc->pClient == pClient) + return pOc; + + return NULL; +} + +/* + * Create an overlay client object for the given client + */ +CompOverlayClientPtr +compCreateOverlayClient (ScreenPtr pScreen, ClientPtr pClient) +{ + CompScreenPtr cs = GetCompScreen(pScreen); + CompOverlayClientPtr pOc; + + pOc = (CompOverlayClientPtr) xalloc(sizeof(CompOverlayClientRec)); + if (pOc == NULL) + return NULL; + + pOc->pClient = pClient; + pOc->pScreen = pScreen; + pOc->resource = FakeClientID(pClient->index); + pOc->pNext = cs->pOverlayClients; + cs->pOverlayClients = pOc; + + /* + * Create a resource for this element so it can be deleted + * when the client goes away. + */ + if (!AddResource (pOc->resource, CompositeClientOverlayType, (pointer) pOc)) + return NULL; + + return pOc; +} + +/* + * Create the overlay window and map it + */ +Bool +compCreateOverlayWindow (ScreenPtr pScreen) +{ + CompScreenPtr cs = GetCompScreen(pScreen); + WindowPtr pRoot = WindowTable[pScreen->myNum]; + WindowPtr pWin; + XID overrideRedirect = TRUE; + int result; + + pWin = cs->pOverlayWin = + CreateWindow (cs->overlayWid, pRoot, + 0, 0, pScreen->width, pScreen->height, 0, + InputOutput, CWOverrideRedirect, &overrideRedirect, + pRoot->drawable.depth, + serverClient, pScreen->rootVisual, &result); + if (pWin == NULL) + return FALSE; + + if (!AddResource(pWin->drawable.id, RT_WINDOW, (pointer)pWin)) + return FALSE; + + MapWindow(pWin, serverClient); + + return TRUE; +} + +/* + * Destroy the overlay window + */ +void +compDestroyOverlayWindow (ScreenPtr pScreen) +{ + CompScreenPtr cs = GetCompScreen(pScreen); + + cs->pOverlayWin = NullWindow; + FreeResource (cs->overlayWid, RT_NONE); +} + -- cgit v1.2.3