Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#8022 closed defect (fixed)

X crashes when Browse views http://wiki.laptop.org/go/Hardware_specification

Reported by: tomeu Owned by: dsd
Priority: blocker Milestone: 8.2.0 (was Update.2)
Component: x window system Version: not specified
Keywords: blocks:8.2.0 8.2-759:+ Cc: morgs, JordanCrouse, jg
Blocked By: Blocking:
Deployments affected: Action Needed: test in release
Verified: no

Description

seems like mozilla requests a pixmap too big for the available memory, then X tries to free the failed pixmap (NULL).

Program received signal SIGSEGV, Segmentation fault.
XvDestroyPixmap (pPix=0x0) at xvmain.c:348
348	  pScreen = pPix->drawable.pScreen;
(gdb) bt
#0  XvDestroyPixmap (pPix=0x0) at xvmain.c:348
#1  0x0808119f in ProcCreatePixmap (client=0x9450950) at dispatch.c:1342
#2  0x08085dbf in Dispatch () at dispatch.c:454
#3  0x0806b63d in main (argc=7, argv=0xbfa7c494, envp=0x0) at main.c:441
(gdb) l
343	  XvAdaptorPtr pa;
344	  int na;
345	  XvPortPtr pp;
346	  int np;
347	
348	  pScreen = pPix->drawable.pScreen;
349	
350	  SCREEN_PROLOGUE(pScreen, DestroyPixmap);
351	
352	  pxvs = (XvScreenPtr)dixLookupPrivate(&pScreen->devPrivates, XvScreenKey);
(gdb) up
#1  0x0808119f in ProcCreatePixmap (client=0x9450950) at dispatch.c:1342
1342	    (*pDraw->pScreen->DestroyPixmap)(pMap);
(gdb) l
1337		    return rc;
1338		}
1339		if (AddResource(stuff->pid, RT_PIXMAP, (pointer)pMap))
1340		    return(client->noClientException);
1341	    }
1342	    (*pDraw->pScreen->DestroyPixmap)(pMap);
1343	    return (BadAlloc);
1344	}
1345	
1346	int

I guess XvDestroyPixmap should check the state of the pointer, but I guess that depends on the X conventions.

We should check if upstream has already fixed this and in that case backport the fix.

Change History (21)

comment:1 Changed 6 years ago by tomeu

Maybe we can ask mozilla not to request too big pixmaps via #7078 , but I guess we want to fix any bugs that kill X.

comment:2 Changed 6 years ago by erikos

I tested this bug with the wikiactivity which has the cache size set to 5000 but it crashed anyway.

comment:3 Changed 6 years ago by morgs

  • Cc morgs added

comment:4 Changed 6 years ago by cjb

Tomeu/Simon -- I wouldn't expect setting the RAM *cache* lower to affect loading an image on a current page. The cache specifies how many images to keep in RAM only.

comment:5 Changed 6 years ago by tomeu

Well, if you have a maximum cache size of 50MB and are already using 48MB, I hope mozilla won't try to allocate a 10MB pixmap. But that may not be how it is implemented, yeah.

comment:6 Changed 6 years ago by dsd

  • Cc dsd added

comment:7 Changed 6 years ago by jg

  • Cc JordanCrouse added

What in the world is Xv doing involved in dealing with that page?

This stinks of something else going on...

comment:8 Changed 6 years ago by jg

  • Action Needed changed from communicate to diagnose

comment:9 Changed 6 years ago by jg

  • Priority changed from high to blocker

comment:10 Changed 6 years ago by jg

  • Owner changed from bernie to jg

comment:11 Changed 6 years ago by gregorio

  • Keywords blocks:8.2.0 added; blocks?:8.2.0 removed

comment:12 Changed 6 years ago by dsd

  • Cc jg added; dsd removed
  • Owner changed from jg to dsd

I'll work on this

comment:13 Changed 6 years ago by dsd

  • Action Needed changed from diagnose to review

avoiding the DestroyPixmap call when there is a NULL pixmap avoids the issue.

Browse still crashes on the Hardware page, but at least it does not kill X, and this is not a regression over 8.1 (browse crashes there too)

Sent an xserver patch upstream:
http://lists.freedesktop.org/archives/xorg/2008-September/038155.html

comment:14 Changed 6 years ago by dsd

  • Action Needed changed from review to test in build

Patch accepted upstream, fixed in xorg-x11-server-1.4.99.906-2.olpc3.3

comment:15 Changed 6 years ago by dsd

  • Action Needed changed from test in build to approve for release

Tested, works fine (Browse crashes like it does in 8.1, but at least it does not kill X)

Please approve xorg-x11-server-1.4.99.906-2.olpc3.3 for 8.2

Changelog:

  • Fix an X server crash when it cannot satisfy a large memory allocation request

|TestCase|

Open http://wiki.laptop.org/go/Hardware_specification in browse, confirm that X does not crash (note that Browse may be unresponsive for a while, this is not a regression)

comment:16 Changed 6 years ago by mstone

  • Action Needed changed from approve for release to add to release

comment:17 Changed 6 years ago by cscott

  • Action Needed changed from add to release to test in release

Added to stable repo; please test in build 758 and following.

comment:18 Changed 6 years ago by dsd

  • Keywords 8.2-759:+ added
  • Resolution set to fixed
  • Status changed from new to closed

Confirmed working in build 759: X no longer crashes.

Instead, Browse freezes for a long time (there are other tickets for this) or it crashes quickly with BadAlloc (#8326). This is the same as it was on update1.

comment:19 follow-up: Changed 6 years ago by frances

tested http on 3 different computers, all connected to the xstest/school server and there was no delay or crash. Smooth sailing.

comment:20 in reply to: ↑ 19 Changed 6 years ago by frances

Replying to frances:

tested http on 3 different computers, all connected to the xstest/school server and there was no delay or crash. Smooth sailing.

tested on 8.2-759

comment:21 Changed 6 years ago by dsd

This isn't fixed, we just made the hardware page images smaller. This page will still exhibit the bug: http://dev.laptop.org/~seth/crashX.html

Note: See TracTickets for help on using tickets.