Attachment 40973 Details for Bug 31501 – Possible fix

[patch] Possible fix

0001-Don-t-free-font-closures-prematurely-31501.patch (text/plain), 8.05 KB, created by James Jones on 2010-12-09 16:27:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: James Jones

Created: 2010-12-09 16:27:42 UTC

Size: 8.05 KB

patch

obsolete

>From 59616dde4a04d407db76d55b06bcc82d646f42cd Mon Sep 17 00:00:00 2001
>From: James Jones <jajones@nvidia.com>
>Date: Thu, 9 Dec 2010 16:14:05 -0800
>Subject: [PATCH] Don't free font closures prematurely (#31501)
>X-NVConfidentiality: public
>
>When doing Xinerama, font ops are dispatched across
>backend screens.  To avoid sleeping a client more than
>once in these cases, logic was added to avoid sleeping and
>destroy the closure structure when the client is already
>asleep.  However, the operation isn't always completed
>when this cleanup is performed.  To freeing prematurely,
>only free the closure data if the operation is finished,
>or if the operation never slept.  The latter catches the
>xinerama case.
>
>Signed-off-by: James Jones <jajones@nvidia.com>
>---
> dix/dixfonts.c     |  102 +++++++++++++++++++++++++++++++++++----------------
> include/closestr.h |    5 +++
> 2 files changed, 75 insertions(+), 32 deletions(-)
>
>diff --git a/dix/dixfonts.c b/dix/dixfonts.c
>index ccb4627..c0ae28b 100644
>--- a/dix/dixfonts.c
>+++ b/dix/dixfonts.c
>@@ -239,6 +239,8 @@ doOpenFont(ClientPtr client, OFclosurePtr c)
>                *newname;
>     int         newlen;
>     int		aliascount = 20;
>+    Bool	fromDispatch = c->from_dispatch;
>+    Bool        finished = FALSE;
>     /*
>      * Decide at runtime what FontFormat to use.
>      */
>@@ -270,6 +272,8 @@ doOpenFont(ClientPtr client, OFclosurePtr c)
> 
> 	BitmapFormatScanlineUnit8;
> 
>+    c->from_dispatch = FALSE;
>+
>     if (client->clientGone)
>     {
> 	if (c->current_fpe < c->num_fpes)
>@@ -374,13 +378,16 @@ bail:
> 			  c->fontid, FontToXError(err));
>     }
>     ClientWakeup(c->client);
>+    finished = TRUE;
> xinerama_sleep:
>-    for (i = 0; i < c->num_fpes; i++) {
>-	FreeFPE(c->fpe_list[i]);
>+    if (finished || fromDispatch) {
>+	for (i = 0; i < c->num_fpes; i++) {
>+	    FreeFPE(c->fpe_list[i]);
>+	}
>+	free(c->fpe_list);
>+	free(c->fontname);
>+	free(c);
>     }
>-    free(c->fpe_list);
>-    free(c->fontname);
>-    free(c);
>     return TRUE;
> }
> 
>@@ -461,6 +468,7 @@ OpenFont(ClientPtr client, XID fid, Mask flags, unsigned lenfname, char *pfontna
>     c->num_fpes = num_fpes;
>     c->fnamelen = lenfname;
>     c->flags = flags;
>+    c->from_dispatch = TRUE;
>     c->non_cachable_font = cached;
> 
>     (void) doOpenFont(client, c);
>@@ -592,6 +600,10 @@ doListFontsAndAliases(ClientPtr client, LFclosurePtr c)
>     char	*bufptr;
>     char	*bufferStart;
>     int		aliascount = 0;
>+    Bool	fromDispatch = c->from_dispatch;
>+    Bool	finished = FALSE;
>+
>+    c->from_dispatch = FALSE;
> 
>     if (client->clientGone)
>     {
>@@ -667,7 +679,7 @@ doListFontsAndAliases(ClientPtr client, LFclosurePtr c)
> 		    ((pointer) c->client, fpe, &name, &namelen, &tmpname,
> 		     &resolvedlen, c->current.private);
> 		if (err == Suspended) {
>-		    if (ClientIsAsleep(client))
>+		    if (!ClientIsAsleep(client))
> 			ClientSleep(client,
> 				    (ClientSleepProcPtr)doListFontsAndAliases,
> 				    c);
>@@ -822,13 +834,16 @@ finish:
> 
> bail:
>     ClientWakeup(client);
>+    finished = TRUE;
> xinerama_sleep:
>-    for (i = 0; i < c->num_fpes; i++)
>-	FreeFPE(c->fpe_list[i]);
>-    free(c->fpe_list);
>-    free(c->savedName);
>-    FreeFontNames(names);
>-    free(c);
>+    if (finished || fromDispatch) {
>+	for (i = 0; i < c->num_fpes; i++)
>+	    FreeFPE(c->fpe_list[i]);
>+	free(c->fpe_list);
>+	free(c->savedName);
>+	FreeFontNames(names);
>+	free(c);
>+    }
>     free(resolved);
>     return TRUE;
> }
>@@ -880,6 +895,7 @@ ListFonts(ClientPtr client, unsigned char *pattern, unsigned length,
>     c->current.list_started = FALSE;
>     c->current.private = 0;
>     c->haveSaved = FALSE;
>+    c->from_dispatch = TRUE;
>     c->savedName = 0;
>     doListFontsAndAliases(client, c);
>     return Success;
>@@ -891,6 +907,8 @@ doListFontsWithInfo(ClientPtr client, LFWIclosurePtr c)
>     FontPathElementPtr fpe;
>     int         err = Successful;
>     char       *name;
>+    Bool        fromDispatch = c->from_dispatch;
>+    Bool        finished = FALSE;
>     int         namelen;
>     int         numFonts;
>     FontInfoRec fontInfo,
>@@ -902,6 +920,8 @@ doListFontsWithInfo(ClientPtr client, LFWIclosurePtr c)
>     int		aliascount = 0;
>     xListFontsWithInfoReply finalReply;
> 
>+    c->from_dispatch = FALSE;
>+
>     if (client->clientGone)
>     {
> 	if (c->current.current_fpe < c->num_fpes)
>@@ -1095,13 +1115,16 @@ finish:
>     WriteSwappedDataToClient(client, length, &finalReply);
> bail:
>     ClientWakeup(client);
>+    finished = TRUE;
> xinerama_sleep:
>-    for (i = 0; i < c->num_fpes; i++)
>-	FreeFPE(c->fpe_list[i]);
>-    free(c->reply);
>-    free(c->fpe_list);
>-    free(c->savedName);
>-    free(c);
>+    if (finished || fromDispatch) {
>+	for (i = 0; i < c->num_fpes; i++)
>+	    FreeFPE(c->fpe_list[i]);
>+	free(c->reply);
>+	free(c->fpe_list);
>+	free(c->savedName);
>+	free(c);
>+    }
>     return TRUE;
> }
> 
>@@ -1150,6 +1173,7 @@ StartListFontsWithInfo(ClientPtr client, int length, unsigned char *pattern,
>     c->current.private = 0;
>     c->savedNumFonts = 0;
>     c->haveSaved = FALSE;
>+    c->from_dispatch = TRUE;
>     c->savedName = 0;
>     doListFontsWithInfo(client, c);
>     return Success;
>@@ -1171,6 +1195,10 @@ doPolyText(ClientPtr client, PTclosurePtr c)
>     FontPathElementPtr fpe;
>     GC *origGC = NULL;
>     int itemSize = c->reqType == X_PolyText8 ? 1 : 2;
>+    Bool fromDispatch = c->from_dispatch;
>+    Bool finished = FALSE;
>+
>+    c->from_dispatch = FALSE;
> 
>     if (client->clientGone)
>     {
>@@ -1417,16 +1445,19 @@ bail:
>     if (ClientIsAsleep(client))
>     {
> 	ClientWakeup(c->client);
>+	finished = TRUE;
> xinerama_sleep:
>-	ChangeGC(NullClient, c->pGC, clearGCmask, clearGC);
>+	if (finished || fromDispatch) {
>+	    ChangeGC(NullClient, c->pGC, clearGCmask, clearGC);
> 
>-	/* Unreference the font from the scratch GC */
>-	CloseFont(c->pGC->font, (Font)0);
>-	c->pGC->font = NullFont;
>+	    /* Unreference the font from the scratch GC */
>+	    CloseFont(c->pGC->font, (Font)0);
>+	    c->pGC->font = NullFont;
> 
>-	FreeScratchGC(c->pGC);
>-	free(c->data);
>-	free(c);
>+	    FreeScratchGC(c->pGC);
>+	    free(c->data);
>+	    free(c);
>+	}
>     }
>     return TRUE;
> }
>@@ -1462,6 +1493,10 @@ doImageText(ClientPtr client, ITclosurePtr c)
>     int err = Success, lgerr;	/* err is in X error, not font error, space */
>     FontPathElementPtr fpe;
>     int itemSize = c->reqType == X_ImageText8 ? 1 : 2;
>+    Bool fromDispatch = c->from_dispatch;
>+    Bool finished = FALSE;
>+
>+    c->from_dispatch = FALSE;
> 
>     if (client->clientGone)
>     {
>@@ -1571,16 +1606,19 @@ bail:
>     if (ClientIsAsleep(client))
>     {
> 	ClientWakeup(c->client);
>+	finished = TRUE;
> xinerama_sleep:
>-	ChangeGC(NullClient, c->pGC, clearGCmask, clearGC);
>+	if (finished || fromDispatch) {
>+	    ChangeGC(NullClient, c->pGC, clearGCmask, clearGC);
> 
>-	/* Unreference the font from the scratch GC */
>-	CloseFont(c->pGC->font, (Font)0);
>-	c->pGC->font = NullFont;
>+	    /* Unreference the font from the scratch GC */
>+	    CloseFont(c->pGC->font, (Font)0);
>+	    c->pGC->font = NullFont;
> 
>-	FreeScratchGC(c->pGC);
>-	free(c->data);
>-	free(c);
>+	    FreeScratchGC(c->pGC);
>+	    free(c->data);
>+	    free(c);
>+	}
>     }
>     return TRUE;
> }
>diff --git a/include/closestr.h b/include/closestr.h
>index ab18ef9..994263f 100644
>--- a/include/closestr.h
>+++ b/include/closestr.h
>@@ -53,6 +53,7 @@ typedef struct _OFclosure {
>     XID         fontid;
>     char       *fontname;
>     int         fnamelen;
>+    Bool	from_dispatch;
>     FontPtr	non_cachable_font;
> }           OFclosureRec;
> 
>@@ -78,6 +79,7 @@ typedef struct _LFWIclosure {
>     LFWIstateRec	saved;
>     int			savedNumFonts;
>     Bool		haveSaved;
>+    Bool		from_dispatch;
>     char		*savedName;
> } LFWIclosureRec;
> 
>@@ -91,6 +93,7 @@ typedef struct _LFclosure {
>     LFWIstateRec current;
>     LFWIstateRec saved;
>     Bool        haveSaved;
>+    Bool	from_dispatch;
>     char	*savedName;
>     int		savedNameLen;
> }	LFclosureRec;
>@@ -109,6 +112,7 @@ typedef struct _PTclosure {
>     CARD8		reqType;
>     XID			did;
>     int			err;
>+    Bool		from_dispatch;
> } PTclosureRec;
> 
> /* ImageText */
>@@ -123,5 +127,6 @@ typedef struct _ITclosure {
>     int			yorg;
>     CARD8		reqType;
>     XID			did;
>+    Bool		from_dispatch;
> } ITclosureRec;
> #endif				/* CLOSESTR_H */
>-- 
>1.7.1
>

Actions: View | Splinter Review

Attachments on bug 31501: 40973 | 118203

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.