[PATCH] Don't free font closures prematurely (#31501)
Jeremy Huddleston
jeremyhu at freedesktop.org
Fri Jul 29 11:23:15 PDT 2011
This patch is attached to https://bugs.freedesktop.org/show_bug.cgi?id=31501 , but so far nobody has bothered to send it to xorg-devel ... I have not tested it and am just forwarding it along for comment since it reportedly addresses this crasher.
From 59616dde4a04d407db76d55b06bcc82d646f42cd Mon Sep 17 00:00:00 2001
From: James Jones <jajones at 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 at 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
More information about the xorg-devel
mailing list