516335 - calling lockImage multiple times for borderImage

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Bobby Holley (:bholley)]

Getting this when I fire up the false/true configuration on my debug build.
ABORT: Only call lockImage once per imgIRequest!: '!mLockHeld', file /files/mozilla/src/modules/libpr0n/src/imgRequestProxy.cpp, line 299


0   libSystem.B.dylib             	0x916d6e42 __kill + 10
1   libSystem.B.dylib             	0x9174923a raise + 26
2   libSystem.B.dylib             	0x91755679 abort + 73
3   libnspr4.dylib                	0x00147981 PR_Assert + 0 (prlog.c:552)
4   libxpcom_core.dylib           	0x0057ab0d Abort(char const*) + 21
5   libxpcom_core.dylib           	0x0057ade1 NS_DebugBreak_P + 657 (nsDebugImpl.cpp:322)
6   libimglib2.dylib              	0x13fffb3c imgRequestProxy::LockImage() + 110 (imgRequestProxy.cpp:300)
7   libgklayout.dylib             	0x12616f83 nsStyleBorder::SetBorderImage(imgIRequest*) + 115 (nsStyleStructInlines.h:63)
8   libgklayout.dylib             	0x12606ee0 nsRuleNode::ComputeBorderData(void*, nsCSSStruct const&, nsStyleContext*, nsRuleNode*, nsRuleNode::RuleDetail, int) + 3786 (nsRuleNode.cpp:4897)
9   libgklayout.dylib             	0x1260e03b nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*, nsRuleData*, nsCSSStruct*) + 2399 (nsStyleStructList.h:134)
10  libgklayout.dylib             	0x1260fa0f nsRuleNode::GetBorderData(nsStyleContext*) + 111 (nsRuleNode.cpp:1649)
11  libgklayout.dylib             	0x1260fb15 nsRuleNode::GetStyleBorder(nsStyleContext*, int) + 213 (nsStyleStructList.h:134)
12  libgklayout.dylib             	0x1261a426 nsStyleContext::GetStyleBorder() + 66 (nsStyleStructList.h:134)
13  libgklayout.dylib             	0x1261c56a nsStyleContext::CalcStyleDifference(nsStyleContext*) + 6532 (nsStyleContext.cpp:452)
14  libgklayout.dylib             	0x12437e4c CaptureChange(nsStyleContext*, nsStyleContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, nsChangeHint) + 40
15  libgklayout.dylib             	0x124397ba nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, int) + 1542 (nsFrameManager.cpp:1265)
16  libgklayout.dylib             	0x1243a08a nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, int) + 3798 (nsFrameManager.cpp:1493)
17  libgklayout.dylib             	0x1243a08a nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, int) + 3798 (nsFrameManager.cpp:1493)
18  libgklayout.dylib             	0x1243a08a nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, int) + 3798 (nsFrameManager.cpp:1493)
19  libgklayout.dylib             	0x1243a1e2 nsFrameManager::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint) + 248 (nsFrameManager.cpp:1534)
20  libgklayout.dylib             	0x123f5d0f nsCSSFrameConstructor::RestyleElement(nsIContent*, nsIFrame*, nsChangeHint) + 371 (nsCSSFrameConstructor.cpp:7846)
21  libgklayout.dylib             	0x123f5f12 nsCSSFrameConstructor::ProcessOneRestyle(nsIContent*, nsReStyleHint, nsChangeHint) + 216 (nsCSSFrameConstructor.cpp:11529)
22  libgklayout.dylib             	0x123f7d48 nsCSSFrameConstructor::ProcessPendingRestyleTable(nsDataHashtable<nsISupportsHashKey, nsCSSFrameConstructor::RestyleData>&) + 296 (nsCSSFrameConstructor.cpp:11630)
23  libgklayout.dylib             	0x123f7e5b nsCSSFrameConstructor::ProcessPendingRestyles() + 157 (nsCSSFrameConstructor.cpp:11666)
24  libgklayout.dylib             	0x1246d7ab PresShell::FlushPendingNotifications(mozFlushType) + 447 (nsPresShell.cpp:4776)
25  libgklayout.dylib             	0x1271ef5e nsDocument::FlushPendingNotifications(mozFlushType) + 430 (nsDocument.cpp:6310)
26  libgklayout.dylib             	0x1242a4ee DocumentViewerImpl::SizeToContent() + 640 (nsDocumentViewer.cpp:3293)
27  libgklayout.dylib             	0x129ead51 nsGlobalWindow::SizeToContent() + 447 (nsGlobalWindow.cpp:4672)
28  libxpcom_core.dylib           	0x005872cb NS_InvokeByIndex_P + 99 (xptcinvoke_unixish_x86.cpp:179)
29  libxpconnect.dylib            	0x152aba24 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) + 6184 (xpcwrappednative.cpp:2710)
30  libxpconnect.dylib            	0x152b7e80 XPC_WN_CallMethod(JSContext*, JSObject*, unsigned int, long*, long*) + 498
31  libmozjs.dylib                	0x00308b0d js_Invoke + 2437 (jsinterp.cpp:1364)
32  libmozjs.dylib                	0x002f35b0 js_Interpret + 102914 (jsops.cpp:2198)
33  libmozjs.dylib                	0x00308b56 js_Invoke + 2510 (jsinterp.cpp:1372)
34  libxpconnect.dylib            	0x152a4c9a nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) + 4758 (xpcwrappedjsclass.cpp:1671)
35  libxpconnect.dylib            	0x1529b7a5 nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) + 97 (xpcwrappedjs.cpp:570)
36  libxpcom_core.dylib           	0x005875ae PrepareAndDispatch(nsXPTCStubBase*, unsigned int, unsigned int*) + 662
37  libxpcom_core.dylib           	0x0058760d nsXPTCStubBase::Stub3() + 53 (xptcstubsdef.inc:1)
38  libgklayout.dylib             	0x12988469 nsXULDocument::DoneWalking() + 1559 (nsXULDocument.cpp:3242)
39  libgklayout.dylib             	0x1298dc26 nsXULDocument::ResumeWalk() + 3526 (nsXULDocument.cpp:3155)
40  libgklayout.dylib             	0x1298e274 nsXULDocument::OnStreamComplete(nsIStreamLoader*, nsISupports*, unsigned int, unsigned int, unsigned char const*) + 1278 (nsXULDocument.cpp:3609)
41  libnecko.dylib                	0x141c7ed3 nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) + 169 (nsStreamLoader.cpp:110)
42  libjar50.dylib                	0x140e7a34 nsJARChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) + 194 (nsJARChannel.cpp:881)
43  libnecko.dylib                	0x14192a32 nsInputStreamPump::OnStateStop() + 272 (nsInputStreamPump.cpp:577)
44  libnecko.dylib                	0x14192b52 nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) + 154 (nsInputStreamPump.cpp:401)
45  libxpcom_core.dylib           	0x0053628a nsInputStreamReadyEvent::Run() + 100 (nsStreamUtils.cpp:113)
46  libxpcom_core.dylib           	0x0056a02c nsThread::ProcessNextEvent(int, int*) + 676 (nsThread.cpp:527)
47  libxpcom_core.dylib           	0x004ee3c0 NS_ProcessPendingEvents_P(nsIThread*, unsigned int) + 146
48  libwidget_mac.dylib           	0x15144811 nsBaseAppShell::NativeEventCallback() + 181 (nsBaseAppShell.cpp:122)
49  libwidget_mac.dylib           	0x150f9244 nsAppShell::ProcessGeckoEvents(void*) + 518 (nsAppShell.mm:414)
50  com.apple.CoreFoundation      	0x903453c5 CFRunLoopRunSpecific + 3141
51  com.apple.CoreFoundation      	0x90345aa8 CFRunLoopRunInMode + 88
52  com.apple.HIToolbox           	0x969942ac RunCurrentEventLoopInMode + 283
53  com.apple.HIToolbox           	0x969940c5 ReceiveNextEventCommon + 374
54  com.apple.HIToolbox           	0x96993f39 BlockUntilNextEventMatchingListInMode + 106
55  com.apple.AppKit              	0x928926d5 _DPSNextEvent + 657
56  com.apple.AppKit              	0x92891f88 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128
57  com.apple.AppKit              	0x9288af9f -[NSApplication run] + 795
58  libwidget_mac.dylib           	0x150f726e nsAppShell::Run() + 288 (nsAppShell.mm:766)
59  libtoolkitcomps.dylib         	0x14e1bdb2 nsAppStartup::Run() + 148 (nsAppStartup.cpp:182)
60  XUL                           	0x000acd33 XRE_main + 14097 (nsAppRunner.cpp:3479)
61  org.mozilla.firefox           	0x000027db main + 709 (nsBrowserApp.cpp:156)
62  org.mozilla.firefox           	0x00001e07 _start + 209
63  org.mozilla.firefox           	0x00001d35 start + 41

Comment 2

[Bobby Holley (:bholley)]

according to bz it is possible to have an imgIRequest shared between multiple nsStyleBorders. So we just need to implement a counted lock. I'll do that in
about an hour.

Comment 3

[Bobby Holley (:bholley)]

Attachment: stopgap patch

adding a stopgap patch (just return true if mLockHeld) I'm about to push to mc. bz gave his r+ on irc before he went to bed.

Comment 4

[Bobby Holley (:bholley)]

pushed stopgap to mc as f2b38abcb709.

Comment 5

[Bobby Holley (:bholley)]

Attachment: patch v1

added a patch to handle this stuff properly. I decided to bring imgRequest into the layered call architecture too, because it was getting too complicated to deal with both deferred requests (if the container doesn't exist yet) and changing owners in imgRequestProxy. To avoid worrying about a lot of edge case interaction between the two, I moved deferred request handling to imgRequest.

flagging joe for review. pushed to try as 0087a781ae37.

Comment 6

[Bobby Holley (:bholley)]

since the stopgap landed, this is no longer a blocker.

Comment 7

[Bobby Holley (:bholley)]

try looks ok except for a random-looking places xpcshell failure on mac.

Comment 8

[Ben Combee (:bcombee)]

Just started seeing this crash on Win32 Fennec using trunk when closing bookmarks list.  Will try the proposed patch and see if it fixes things.

Comment 9

[Ben Combee (:bcombee)]

With the patch applied, I no longer crash Fennec.

Comment 10

[Joe Drew (not getting mail)]

Comment on attachment 400696 [details] [diff] [review]
patch v1

>-  // If we're holding a lock, unlock the old image
>-  PRBool wasLocked = mLockHeld;
>-  if (mLockHeld)
>+  // If we're holding locks, unlock the old image
>+  PRUint32 oldLockCount = mLocksHeld;
>+  while (mLocksHeld)
>     UnlockImage();

This, without looking at UnlockImage(), looks like an infinite loop. Maybe add another function that unlocks all locks in a straightforward way?

Comment 11

[Bobby Holley (:bholley)]

Attachment: patch v2

I don't think a separate method is necessary - just added a comment above the two while loops instead.

I'll push this once my plane lands.

Comment 12

[Bobby Holley (:bholley)]

pushed to mc e8156cb0f14a. resolving fixed.