[PATCH 3/3] Reset only what the protocol requires, instead of full regeneration.
Jeremy Huddleston
jeremyhu at apple.com
Tue Nov 22 09:37:09 PST 2011
I'd rather deprecate serverGeneration in this build and remove it in the next. In addition to not breaking ABI, this gives a compiler warning about the deprecation which removing the symbol doesn't:
misc.h:
extern _X_EXPORT _X_DEPRECATED unsigned long serverGeneration;
globals.c:
unsigned long serverGeneration = 1;
---
ResetProperties() looks a bit over-complicated to me, and it looks like it can break the linked list because you don't set the previous' next. How about:
void
ResetProperties(WindowPtr pWin)
{
PropertyPtr prev, cur, next;
if (!pWin->optional)
return;
for (prev = NULL, cur = pWin->optional->userProps; cur; cur = next)
{
next = cur->next;
if (cur->retain && ValidAtom(cur->propertyName) && ValidAtom(cur->type))
{
prev = cur;
continue;
}
if (prev)
{
prev->next = next;
} else {
pWin->optional->userProps = next;
}
free(cur->data);
dixFreeObjectWithPrivates(cur, PRIVATE_PROPERTY);
}
}
On Nov 20, 2011, at 03:48, Jamey Sharp wrote:
> It's difficult to get regeneration correct in all the drivers,
> extensions, and everywhere else it's currently required--and there's no
> significant advantage to doing full regeneration. So instead, make DIX
> reset exactly the things required by the protocol specification, and set
> serverGeneration to a constant 1.
>
> Eliminating all the ugly code that's conditional on serverGeneration is
> left as a later exercise. This patch should simply improve reliability
> when the server resets.
>
> Signed-off-by: Jamey Sharp <jamey at minilop.net>
> ---
> This patch is generated with `diff -w`, so the patch's indentation is
> all wrong in places. I believe the real changes are easier to review
> this way, because I had to re-indent most of dix/main.c. If you want to
> review the version with correct indentation, please review it by cloning
> my public xserver repo or by browsing it in cgit:
>
> http://cgit.freedesktop.org/~jamey/xserver/commit/?h=no-regen&id=8fd86ed43310863d7c3321c007d9cf6419b8c120
>
> dix/atom.c | 30 +++++++++++++++++
> dix/devices.c | 22 ++++++++++++
> dix/globals.c | 1 -
> dix/main.c | 88 +++++++++++++++++++++++++++++++++-----------------
> dix/property.c | 21 ++++++++++++
> dix/resource.c | 3 +-
> dix/window.c | 13 +++++--
> include/dix.h | 7 ++++
> include/misc.h | 2 +-
> include/property.h | 2 +
> include/propertyst.h | 1 +
> include/window.h | 3 ++
> 12 files changed, 156 insertions(+), 37 deletions(-)
>
> diff --git a/dix/atom.c b/dix/atom.c
> index 88b40db..5eb1d55 100644
> --- a/dix/atom.c
> +++ b/dix/atom.c
> @@ -68,6 +68,7 @@ typedef struct _Node {
> } NodeRec, *NodePtr;
>
> static Atom lastAtom = None;
> +static Atom lastServerAtom = None;
> static NodePtr atomRoot = NULL;
> static unsigned long tableLength;
> static NodePtr *nodeTable;
> @@ -201,6 +202,7 @@ FreeAllAtoms(void)
> free(nodeTable);
> nodeTable = NULL;
> lastAtom = None;
> + lastServerAtom = None;
> }
>
> void
> @@ -216,3 +218,31 @@ InitAtoms(void)
> if (lastAtom != XA_LAST_PREDEFINED)
> AtomError();
> }
> +
> +void
> +SaveServerAtoms(void)
> +{
> + lastServerAtom = lastAtom;
> +}
> +
> +static void
> +ResetAtom(NodePtr *patom)
> +{
> + if (!*patom)
> + return;
> + if ((*patom)->a > lastServerAtom)
> + {
> + FreeAtom(*patom);
> + *patom = NULL;
> + return;
> + }
> + ResetAtom(&(*patom)->left);
> + ResetAtom(&(*patom)->right);
> +}
> +
> +void
> +ResetAtoms(void)
> +{
> + lastAtom = lastServerAtom;
> + ResetAtom(&atomRoot);
> +}
> diff --git a/dix/devices.c b/dix/devices.c
> index da817a8..7d91416 100644
> --- a/dix/devices.c
> +++ b/dix/devices.c
> @@ -2645,3 +2645,25 @@ void valuator_set_mode(DeviceIntPtr dev, int axis, int mode)
> dev->valuator->axes[i].mode = mode;
> }
> }
> +
> +void ResetInput(void)
> +{
> + BITS32 mask = KBKeyClickPercent | KBBellPercent | KBBellPitch | KBBellDuration | KBAutoRepeatMode;
> + XID attrs[] = { -1, -1, -1, -1, AutoRepeatModeDefault };
> + DeviceIntPtr dev;
> + for (dev = inputInfo.devices; dev; dev = dev->next)
> + {
> + /* TODO: reset keyboard and pointer mappings */
> + if (dev->focus)
> + {
> + dev->focus->win = PointerRootWin;
> + dev->focus->revert = None;
> + dev->focus->time = currentTime;
> + dev->focus->traceGood = 0;
> + }
> + if (dev->kbdfeed)
> + DoChangeKeyboardControl(serverClient, dev, attrs, mask);
> + if (dev->ptrfeed)
> + dev->ptrfeed->ctrl = defaultPointerControl;
> + }
> +}
> diff --git a/dix/globals.c b/dix/globals.c
> index 0a6b170..28f9dbd 100644
> --- a/dix/globals.c
> +++ b/dix/globals.c
> @@ -84,7 +84,6 @@ int currentMaxClients; /* current size of clients array */
> long maxBigRequestSize = MAX_BIG_REQUEST_SIZE;
>
> unsigned long globalSerialNumber = 0;
> -unsigned long serverGeneration = 0;
>
> /* these next four are initialized in main.c */
> CARD32 ScreenSaverTime;
> diff --git a/dix/main.c b/dix/main.c
> index 16575ce..745fe05 100644
> --- a/dix/main.c
> +++ b/dix/main.c
> @@ -149,9 +149,7 @@ int main(int argc, char *argv[], char *envp[])
>
> alwaysCheckForInput[0] = 0;
> alwaysCheckForInput[1] = 1;
> - while(1)
> - {
> - serverGeneration++;
> +
> ScreenSaverTime = defaultScreenSaverTime;
> ScreenSaverInterval = defaultScreenSaverInterval;
> ScreenSaverBlanking = defaultScreenSaverBlanking;
> @@ -164,18 +162,14 @@ int main(int argc, char *argv[], char *envp[])
> InitBlockAndWakeupHandlers();
> /* Perform any operating system dependent initializations you'd like */
> OsInit();
> - if(serverGeneration == 1)
> - {
> CreateWellKnownSockets();
> +
> for (i=1; i<MAXCLIENTS; i++)
> clients[i] = NullClient;
> serverClient = calloc(sizeof(ClientRec), 1);
> if (!serverClient)
> FatalError("couldn't create server client");
> InitClient(serverClient, 0, (pointer)NULL);
> - }
> - else
> - ResetWellKnownSockets ();
> clients[0] = serverClient;
> currentMaxClients = 1;
>
> @@ -274,6 +268,8 @@ int main(int argc, char *argv[], char *envp[])
> }
> }
>
> + SaveServerAtoms();
> +
> #ifdef XQUARTZ
> /* Let the other threads know the server is done with its init */
> pthread_mutex_lock(&serverRunningMutex);
> @@ -284,8 +280,58 @@ int main(int argc, char *argv[], char *envp[])
>
> NotifyParentProcess();
>
> + while ((dispatchException & DE_TERMINATE) == 0)
> + {
> Dispatch();
>
> + /* As specified in "X Window System Protocol" chapter 10,
> + * "Connection Close":
> + *
> + * At every transition to the state of having no connections as
> + * a result of a connection closing with a Destroy close-down
> + * mode, the server resets its state as if it had just been
> + * started. */
> +
> + /* - This starts by destroying all lingering resources from
> + * clients that have terminated in RetainPermanent or
> + * RetainTemporary mode. */
> +#ifdef PANORAMIX
> + {
> + Bool remember_it = noPanoramiXExtension;
> + noPanoramiXExtension = TRUE;
> + FreeAllResources();
> + noPanoramiXExtension = remember_it;
> + }
> +#else
> + FreeAllResources();
> +#endif
> +
> + /* - It additionally includes deleting all but the predefined
> + * atom identifiers, */
> + ResetAtoms();
> +
> + for (i = 0; i < screenInfo.numScreens; i++)
> + {
> + /* - deleting all properties on all root windows, */
> + ResetProperties(screenInfo.screens[i]->root);
> +
> + /* - restoring the standard root tiles and cursors, */
> + ResetRootWindow(screenInfo.screens[i]->root);
> + }
> +
> + /* - resetting the access control list, */
> + ResetWellKnownSockets();
> +
> + /* - restoring the default font path, */
> + if (SetDefaultFontPath(defaultFontPath) != Success)
> + ErrorF("[dix] failed to set default font path '%s'", defaultFontPath);
> +
> + /* - resetting all device maps and attributes (key click, bell
> + * volume, acceleration),
> + * - and restoring the input focus to state PointerRoot. */
> + ResetInput();
> + }
> +
> #ifdef XQUARTZ
> /* Let the other threads know the server is no longer running */
> pthread_mutex_lock(&serverRunningMutex);
> @@ -301,16 +347,7 @@ int main(int argc, char *argv[], char *envp[])
> FreeScreenSaverTimer();
> CloseDownExtensions();
>
> -#ifdef PANORAMIX
> - {
> - Bool remember_it = noPanoramiXExtension;
> - noPanoramiXExtension = TRUE;
> - FreeAllResources();
> - noPanoramiXExtension = remember_it;
> - }
> -#else
> - FreeAllResources();
> -#endif
> + FreeClientResources(serverClient);
>
> CloseInput();
>
> @@ -338,22 +375,13 @@ int main(int argc, char *argv[], char *envp[])
>
> FreeAuditTimer();
>
> - if (dispatchException & DE_TERMINATE)
> - {
> CloseWellKnownConnections();
> - }
> -
> - OsCleanup((dispatchException & DE_TERMINATE) != 0);
>
> - if (dispatchException & DE_TERMINATE)
> - {
> - ddxGiveUp(EXIT_NO_ERROR);
> - break;
> - }
> + OsCleanup(TRUE);
>
> free(ConnectionInfo);
> - ConnectionInfo = NULL;
> - }
> +
> + ddxGiveUp(EXIT_NO_ERROR);
> return 0;
> }
>
> diff --git a/dix/property.c b/dix/property.c
> index a1ae530..87b8fe3 100644
> --- a/dix/property.c
> +++ b/dix/property.c
> @@ -284,6 +284,7 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
> pProp->format = format;
> pProp->data = data;
> pProp->size = len;
> + pProp->retain = (pClient == serverClient);
> rc = XaceHookPropertyAccess(pClient, pWin, &pProp,
> DixCreateAccess|DixWriteAccess);
> if (rc != Success) {
> @@ -429,6 +430,26 @@ DeleteAllWindowProperties(WindowPtr pWin)
> pWin->optional->userProps = NULL;
> }
>
> +void
> +ResetProperties(WindowPtr pWin)
> +{
> + PropertyPtr tmp, *pProp;
> + if (!pWin->optional)
> + return;
> +
> + for (pProp = &pWin->optional->userProps; (tmp = *pProp); )
> + {
> + if (tmp->retain && ValidAtom(tmp->propertyName) && ValidAtom(tmp->type))
> + {
> + pProp = &tmp->next;
> + continue;
> + }
> + *pProp = tmp->next;
> + free(tmp->data);
> + dixFreeObjectWithPrivates(tmp, PRIVATE_PROPERTY);
> + }
> +}
> +
> static int
> NullPropertyReply(
> ClientPtr client,
> diff --git a/dix/resource.c b/dix/resource.c
> index eb9f049..efe1964 100644
> --- a/dix/resource.c
> +++ b/dix/resource.c
> @@ -863,7 +863,8 @@ FreeAllResources(void)
> {
> int i;
>
> - for (i = currentMaxClients; --i >= 0; )
> + /* don't free serverClient's resources */
> + for (i = currentMaxClients; --i > 0; )
> {
> if (clientTable[i].buckets)
> FreeClientResources(clients[i]);
> diff --git a/dix/window.c b/dix/window.c
> index c87020d..987bb33 100644
> --- a/dix/window.c
> +++ b/dix/window.c
> @@ -543,16 +543,21 @@ void
> InitRootWindow(WindowPtr pWin)
> {
> ScreenPtr pScreen = pWin->drawable.pScreen;
> - int mask = CWBackPixmap | CWBackingStore | CWCursor;
> - XID attrs[] = { None, defaultBackingStore, None };
> -
> if (!(*pScreen->CreateWindow)(pWin))
> return; /* XXX */
> (*pScreen->PositionWindow)(pWin, 0, 0);
> - ChangeWindowAttributes(pWin, mask, attrs, serverClient);
> + ResetRootWindow(pWin);
> MapWindow(pWin, serverClient);
> }
>
> +void
> +ResetRootWindow(WindowPtr pWin)
> +{
> + int mask = CWBackPixmap | CWBackingStore | CWCursor;
> + XID attrs[] = { None, defaultBackingStore, None };
> + ChangeWindowAttributes(pWin, mask, attrs, serverClient);
> +}
> +
> /* Set the region to the intersection of the rectangle and the
> * window's winSize. The window is typically the parent of the
> * window from which the region came.
> diff --git a/include/dix.h b/include/dix.h
> index 34661f3..7a931c3 100644
> --- a/include/dix.h
> +++ b/include/dix.h
> @@ -306,12 +306,19 @@ extern _X_EXPORT void FreeAllAtoms(void);
>
> extern _X_EXPORT void InitAtoms(void);
>
> +extern void SaveServerAtoms(void);
> +extern void ResetAtoms(void);
> +
> /* main.c */
>
> extern _X_EXPORT void SetVendorRelease(int release);
>
> extern _X_EXPORT void SetVendorString(char *string);
>
> +/* devices.c */
> +
> +extern void ResetInput(void);
> +
> /* events.c */
>
> extern void SetMaskForEvent(
> diff --git a/include/misc.h b/include/misc.h
> index dc03911..1097050 100644
> --- a/include/misc.h
> +++ b/include/misc.h
> @@ -357,6 +357,6 @@ typedef struct _CharInfo *CharInfoPtr; /* also in fonts/include/font.h */
> #endif
>
> extern _X_EXPORT unsigned long globalSerialNumber;
> -extern _X_EXPORT unsigned long serverGeneration;
> +#define serverGeneration 1
>
> #endif /* MISC_H */
> diff --git a/include/property.h b/include/property.h
> index 075eb4a..01379ed 100644
> --- a/include/property.h
> +++ b/include/property.h
> @@ -88,4 +88,6 @@ extern _X_EXPORT int DeleteProperty(
> extern _X_EXPORT void DeleteAllWindowProperties(
> WindowPtr /*pWin*/);
>
> +extern void ResetProperties(WindowPtr /*pWin*/);
> +
> #endif /* PROPERTY_H */
> diff --git a/include/propertyst.h b/include/propertyst.h
> index 1edd11d..73ccce6 100644
> --- a/include/propertyst.h
> +++ b/include/propertyst.h
> @@ -61,6 +61,7 @@ typedef struct _Property {
> uint32_t format; /* format of data for swapping - 8,16,32 */
> uint32_t size; /* size of data in (format/8) bytes */
> pointer data; /* private to client */
> + unsigned int retain : 1; /* should this property be retained across server reset? */
> PrivateRec *devPrivates;
> } PropertyRec;
>
> diff --git a/include/window.h b/include/window.h
> index e13598b..1bb51e7 100644
> --- a/include/window.h
> +++ b/include/window.h
> @@ -93,6 +93,9 @@ extern _X_EXPORT Bool CreateRootWindow(
> extern _X_EXPORT void InitRootWindow(
> WindowPtr /*pWin*/);
>
> +extern void ResetRootWindow(
> + WindowPtr /*pWin*/);
> +
> typedef WindowPtr (* RealChildHeadProc) (WindowPtr pWin);
>
> extern _X_EXPORT void RegisterRealChildHeadProc (RealChildHeadProc proc);
> --
> 1.7.5.4
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
More information about the xorg-devel
mailing list