This patch isn't a correct fix. It just avoids the crash. I wish I could remember why. There was some discussion about this on IRC between Adam Jackson and I a long time ago that basically concluded the correct fix is "really hard." If anyone keeps really old IRC logs around (around 8 or 9 months back), that would have made a good update in the bug report. Sorry I didn't follow up.
Thanks, -James nvpublic On 7/29/11 11:23 AM, "Jeremy Huddleston" <jerem...@freedesktop.org> wrote: >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 <jajo...@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 <jajo...@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 > _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel