[PATCH 3/3] Reset only what the protocol requires, instead of full regeneration.

Jamey Sharp jamey at minilop.net
Sun Nov 20 03:48:54 PST 2011


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



More information about the xorg-devel mailing list