Closed Bug 829954 Opened 12 years ago Closed 11 years ago

OOM crash in mozilla::gfx::AlphaBoxBlur::Blur

Categories

(Core :: Graphics, defect, P1)

19 Branch
x86
Windows 7
defect

Tracking

()

VERIFIED FIXED
mozilla24
Tracking Status
firefox18 --- unaffected
firefox19 + wontfix
firefox20 --- wontfix
firefox21 + wontfix
firefox22 + verified
firefox23 + verified
firefox24 --- verified

People

(Reporter: scoobidiver, Assigned: bas.schouten, NeedInfo)

References

Details

(4 keywords, Whiteboard: [MemShrink:P1])

Crash Data

Attachments

(3 files, 1 obsolete file)

It's #44 browser crasher in 19.0b1 and #47 in 21.0a1. It first showed up in 19.0a1/20121109. The regression range is: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=36e99ea02c05&tochange=90cea19e27e2 It might be caused by bug 685012. Signature mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur() More Reports Search UUID 2f581d2a-9da6-4665-9f46-7215d2130112 Date Processed 2013-01-12 07:44:56 Uptime 2261 Last Crash 18.1 hours before submission Install Age 21.2 hours since version was first installed. Install Time 2013-01-11 10:28:58 Product Firefox Version 21.0a1 Build ID 20130110030939 Release Channel nightly OS Windows NT OS Version 5.1.2600 Service Pack 2 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 8 stepping 3 Crash Reason EXCEPTION_BREAKPOINT Crash Address 0xf519a7 App Notes AdapterVendorID: 0x10c8, AdapterDeviceID: 0x0016, AdapterSubsysID: 00000000, AdapterDriverVersion: 5.1.2001.0 D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- EMCheckCompatibility True Adapter Vendor ID 0x10c8 Adapter Device ID 0x0016 Total Virtual Memory 2147352576 Available Virtual Memory 1688936448 System Memory Use Percentage 90 Available Page File 7081984 Available Physical Memory 13189120 OOMAllocationSize 1277804 Frame Module Signature Source 0 mozalloc.dll mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:30 1 mozalloc.dll mozalloc_handle_oom memory/mozalloc/mozalloc_oom.cpp:27 2 mozalloc.dll moz_xmalloc memory/mozalloc/mozalloc.cpp:56 3 gkmedias.dll mozilla::gfx::AlphaBoxBlur::Blur gfx/2d/Blur.cpp:525 4 xul.dll gfxAlphaBoxBlur::Paint gfx/thebes/gfxBlur.cpp:86 5 xul.dll nsContextBoxBlur::DoPaint layout/base/nsCSSRendering.cpp:4792 6 xul.dll nsCSSRendering::PaintBoxShadowOuter layout/base/nsCSSRendering.cpp:1351 7 xul.dll nsDisplayBoxShadowOuter::Paint layout/base/nsDisplayList.cpp:2432 8 xul.dll mozilla::FrameLayerBuilder::DrawThebesLayer layout/base/FrameLayerBuilder.cpp:3336 9 xul.dll mozilla::layers::BasicShadowableThebesLayer::PaintBuffer gfx/layers/basic/BasicThebesLayer.cpp:403 10 xul.dll mozilla::layers::BasicThebesLayer::PaintThebes gfx/layers/basic/BasicThebesLayer.cpp:190 More reports at: https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+mozalloc_handle_oom%28unsigned+int%29+|+moz_xmalloc+|+mozilla%3A%3Agfx%3A%3AAlphaBoxBlur%3A%3ABlur%28%29
This report has more stack frames: bp-efb6d972-62e9-4336-b941-7b7022130112 It seems unlikely that bug 685012 (CSS page-break layout code) is the culprit. Bug 807925 would probably be my first guess in that range since it appears to have something to do with painting and images.
Crash Signature: [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()] → [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()] [@ mozalloc_abort(char const* const) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()]
Depends on: 834256
As the STR in bug 834256 report this is happening with viewing PDFs and AFAIK we intend to ship PDF.js with 19, I'm nominating this. When I run across a site making various things available as PDFs, I'm quite likely to open multiple ones at once in multiple tabs, I guess there's a good collection of other people out there probably doing the same, and we should crash in that case as those STR suggest we do.
(In reply to Scoobidiver from comment #2) > Here's an interesting crash report(see User Comments and App Notes): > bp-25827a02-8569-48e0-bcdf-2349a2130119. The App Notes in the UI has been cut off because of the length. Here's the full field as seen in raw JSON: Notes": "AdapterVendorID: 0x8086, AdapterDeviceID: 0x2a42, AdapterSubsysID: 3a0217aa, AdapterDriverVersion: 8.15.10.2302\nD2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ Successful print.\nPrint object tree:\n ad41680 = { mFrameType = 0, mHasBeenPrinted = true, mDontPrint = false, mPrintAsIs = false, mInvisible = false, mPrintPreview = false, mDidCreateDocShell = true, mShrinkRatio = 1, mZoomRatio = 0.9, mContent = null }\nSuccessful print.\nPrint object tree:\n 880f940 = { mFrameType = 0, mHasBeenPrinted = true, mDontPrint = false, mPrintAsIs = false, mInvisible = false, mPrintPreview = false, mDidCreateDocShell = true, mShrinkRatio = 1, mZoomRatio = 0.9, mContent = null }\n 1a9a91c0 = { mFrameType = 1, mHasBeenPrinted = false, mDontPrint = true, mPrintAsIs = true, mInvisible = true, mPrintPreview = false, mDidCreateDocShell = false, mShrinkRatio = 1, mZoomRatio = 1, mContent = iframe }\nSuccessful print.\nPrint object tree:\n 363edc80 = { mFrameType = 0, mHasBeenPrinted = true, mDontPrint = false, mPrintAsIs = false, mInvisible = false, mPrintPreview = false, mDidCreateDocShell = true, mShrinkRatio = 1, mZoomRatio = 0.9, mContent = null }\nSuccessful print.\nPrint object tree:\n 20734f00 = { mFrameType = 0, mHasBeenPrinted = true, mDontPrint = false, mPrintAsIs = false, mInvisible = false, mPrintPreview = false, mDidCreateDocShell = true, mShrinkRatio = 1, mZoomRatio = 0.9, mContent = null }\nPrint object tree:\n 1a76d140 = { mFrameType = 0, mHasBeenPrinted = false, mDontPrint = true, mPrintAsIs = false, mInvisible = false, mPrintPreview = false, mDidCreateDocShell = true, mShrinkRatio = 1, mZoomRatio = 1, mContent = null }\n 1d062340 = { mFrameType = 1, mHasBeenPrinted = false, mDontPrint = true, mPrintAsIs = false, mInvisible = false, mPrintPreview = false, mDidCreateDocShell = false, mShrinkRatio = 1, mZoomRatio = 1, mContent = iframe }\n",
Also, this has been rising in 19 data since 19.0b3 was released.
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #5) > Also, this has been rising in 19 data since 19.0b3 was released. It's indeed #25 top browser crasher in 19.0b3.
Marking tracking so we can watch this in b4 results which should be coming in soon.
Assignee: nobody → karlt
20121109 was one day after bug 509052 landed.
Assignee: karlt → bas
Blocks: 509052
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #3) > As the STR in bug 834256 report this is happening with viewing PDFs and > AFAIK we intend to ship PDF.js with 19, I'm nominating this. Bug 748923 implies that PDF.js should not be enabled on beta.
(In reply to Karl Tomlinson (:karlt) from comment #9) > Bug 748923 implies that PDF.js should not be enabled on beta. But it is, AFAIK, and as of now, there's (from what I heard) no intent to disable it before release of 19, unless Something Bad Happens™.
(In reply to Karl Tomlinson (:karlt) from comment #8) > 20121109 was one day after bug 509052 landed. That bug would've changed the signature so I wonder if there's simply an equivalent crash from before that bug.
We should grab URLs and get some QA around this, since the engineering investigation hasn't turned much up yet.
bp-25827a02-8569-48e0-bcdf-2349a2130119 had an allocation request of 93 MiB, but not all reports have requests this large. Many times as much virtual and physical memory is reported as available, but perhaps these numbers are not what I thought they were. Would checking a fallible allocation request and falling back to the other path use less memory?
http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Familly_Regist http://www.facebook.com/ https://www.facebook.com/ http://web.ebuddy.com/?startsession=1# http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ http://www.detik.com/ http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ http://apps.facebook.com/criminalcase/?fb_source=bookmark_apps&ref=bookmarks&cou http://digsitevalue.net/s/dogma.hr http://mashughuli.blogspot.com/2013/01/hellen-urio-rahma-kitchen-party.html#more http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ http://www.cartoonclub-th.com/2011/12/berserk-241-250.html http://9gag.com/hot http://br46.tribalwars.com.br/game.php?village=305818&screen=main http://ssc.nic.in/press-release/CHSL_2013_ZEROS.pdf http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ http://www.bien.hu/hazi-fogfeherites-termeszetesen,szepseg,arc,111314 http://9gag.com/ http://www.gardenshop.ru/products/Roses_spring_2013/?arrFilter_cf%5B1%5D%5BLEFT% http://www.qcdriver.com/jobs/indeed/oo-tank-reg-otr-chicago.html?utm_source=inde http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ https://twitter.com/login http://www.linhadefensiva.org/forum/topic/148169-chave-de-registro-infectada-rem http://www.nejm.org/doi/pdf/10.1056/NEJMe1110900 http://www.tumblr.com/dashboard http://www.betlighting.com.mx/PDF/Beghelli.pdf http://internet-positif.org/site.block/MThzY2hvb2xnaXJsei5jb20%3D http://www.berniaga.com/Laptop+Notebook+TOSHIBA+Dynabook-11810700.htm https://twitter.com/download/?lang=pt&logged_out=1 http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ http://hwd.up.nic.in/pensioner%20list/bareilly.pdf http://www.n-dorphin.wowstead.com/ http://www.bild.de/ http://mashughuli.blogspot.com/2013/02/jamimas-night-pj-hall-kinondoni.html#more http://magals.tumblr.com/page/16 http://bet-2013.blogspot.com/b/layout-preview?token=aWILuDwBAAA.k-NG7LTJZe0FVjw5 http://www.cartoonclub-th.com/2011/07/toriko-81-90.html http://www.jasolar.com/uploads/files/201212/20121211153830_KHRi5P.pdf wyciwyg://73923/http://lax1.ib.adnxs.com/if?enc=AAAAAAAAFEAAAAAAAAAUQAAAAMDMzAhA http://revistas.inpi.gov.br/pdf/marcas1970.pdf http://www.facebook.com/ai.php?aed=AQL1Ba2Vv2UHjTF6rPqgHa9qVvY9KKkbnuq_KUjOCAghJ wyciwyg://35179/http://fanfiction.mugglenet.com/browse.php?type=titles https://www.facebook.com/ajax/pagelet/generic.php/PhotoViewerInitPagelet?ajaxpip http://akcontent.ebuddy.com/web_banners/invocation.html?z=895&t=1:1;2:34;3:BR&w= http://sssm.nic.in/DataManagement/SamagraData/Pages/Rural_NPR_KYC_Family_Member_ https://www.google.co.id/search?hl=id&site=imghp&tbm=isch&source=hp&biw=1152&bih http://animeid.tv/ver/blood-49 http://www.ncbi.nlm.nih.gov/protein/347840211?report=fasta http://report.lpse.jabarprov.go.id/home.php?click=s1k04ktie8f0dl44gvbp30b3uds962 http://platform.twitter.com/widgets/tweet_button.html?show_screen_name=false&tex http://hollyjadepeers.blogspot.com/?zx=19cb447f51d9085e http://play-therapy.com/playfulpooch/images_resources/APDT.EmotionalDog.pdf http://oasc12.247realmedia.com/RealMedia/ads/adstream_sx.ads/pu http://www.ehow.com/how_8415841_plan-home-cctv-camera-system.html http://aptransport.org/html/form26a.htm http://view.atdmt.com/MRT/iview/436906037/direct/01/20130206174 http://www.milaiwang.com/2013/02/blog-post_2576.html http://www.fanfiction.net/s/6099883/3/Now-And-The-Past https://www.facebook.com/ajax/home/generic.php?ajaxpipe=1&ajaxpipe_token=AXhm6ht http://www.section508.gov/docs/pdfguidanceforgovernment.pdf http://motherless.com/V76764F7 http://www.kaskus.co.id/thread/000000000000000011134756/freestyle-casing-section http://www.rusalarm.ru/assets/files/Starline/sl_a8_red3.pdf http://www.posterheroes.org/category/blog/ http://www.sonorestaurant.com.au/media/pdfs/DinnerMenuNov.pdf http://www.havator.com/media/files/news-magazines/havator_wind_eng.pdf https://ox-social.bidsystem.com/w/1.0/afr?auid=349675&cb=1360241712&ep=tQdN0_58p http://www.facebook.com/plugins/like.php?api_key=&locale=en_US&sdk=joey&channel_ http://h.cartoonclub-th.com/2013/02/i-friggin-hate-boobs.html http://www.facebook.com/ajax/pagelet/generic.php/MoreStoriesPagelet?ajaxpipe=1&a http://edge.omnitwig.com/setImpData.html?p=CPX&i=1&a=Brain&c=471caa2f-32ee-4cb5- http://www.texa.com/gfx/depliant/pieghevole_KONFORT_600E_en-GB.pdf https://play.google.com/apps/publish/v2/?dev_acc=08895330968769390461#MarketList http://9gag.com/trending https://plusone.google.com/_/+1/fastbutton?bsv&size=medium&annotation=none&hl=en http://www.google.gr/#hl=el&tbo=d&sclient=psy-ab&q=%CE%98%CE%9F%CE%A1%CE%A5%CE%9 http://www.emmaus-reisen.de/images/pdf/katalog_web.pdf http://www.homeaway.co.uk/England/holiday-barn-Surrey/p62385.htm#calendar https://www.google.com.pk/ http://pauli.uni-muenster.de/tp/fileadmin/lehre/vorlesungen/klasen/Physik3/kohl1 http://www.facebook.com/ajax/pagelet/generic.php/MoreStoriesPagelet?ajaxpipe=1&a http://www.tochigi-agri.or.jp/shunosoudsan/pdf/kinyurei.pdf http://assets.tumblr.com/analytics.html?459463e736f5e14a6b709883439e6021 https://www.facebook.com/ai.php?aed=AQIj-oRR5-qW0CKx5O1rymTeRdR4KAIevkLmlPO9ia3j https://customer.onlinelic.in/LICEPS/resources/pdf/PrmPayRcpt-MSBI2912430896.pdf http://www.orkut.com.br/Main#Conversations http://www.fold3.com/document/271896294/ http://www.reverbnation.com/naisflavia http://submit-jxb.oxfordjournals.org/submission/pdf?msid=JEXBOT/2013/094433&role https://www.facebook.com/ajax/ei.php?aed=AT50F2oH7rkjqu7v5vKmFvLsNogpDukQnfPjxG8 http://educ.lfhk.cuni.cz/file.php/43/G-12-11-20/12_gen-Light_Electron_Micro.pdf http://www.yophim.com/2012/06/tinh-cam.html http://www.redtube.com/ http://www.facebook.com/ http://lamasat-smsmia.blogspot.com/2011/04/blog-post_12.html http://ficheros.esri.es/conferencia2011/conferencia/medioambiente/imperialcolleg http://www.whitepages.com/name/Carl-D-Reinhardt/North-Sioux-City-
Keywords: needURLs
(In reply to juan becerra [:juanb] from comment #14) > http://sssm.nic.in/DataManagement/SamagraData/Pages/ > Rural_NPR_KYC_Familly_Regist > http://www.facebook.com/ > https://www.facebook.com/ > http://web.ebuddy.com/?startsession=1# > http://sssm.nic.in/DataManagement/SamagraData/Pages/ > Rural_NPR_KYC_Family_Member_ > ... Hae we attempted to reproduce?
I've been trying to reproduce this using the information in comment #2 and the URLs in comment #14. It looks like people are opening PDFs, and I have been trying to: - Open within the browser, using Firefox PDF Preview by default - Open within the browser, using Adobe Reader - Opening in a separate Adobe Reader window And trying thins like printing, but so far no luck.
Based on Bug 834256, you can crash Firefox by opening multiple tabs (6-14) with http://people.mozilla.com/~ydelendik/bug834256/web/viewer.html web page and wait.
That might be a different crash. Opening multiple pdf documents in tabs crashes the browser and yields an empty report. This might be something else.
This seems to be in the <= 4kx4k branch of the code. How hard is the "if larger than 1 << 24, go to a different branch of the code"? Just a magic number assumed to be reasonably small, or is there a hard boundary at 4kx4k (1<<24)?
(In reply to Milan Sreckovic [:milan] from comment #19) > This seems to be in the <= 4kx4k branch of the code. How hard is the "if > larger than 1 << 24, go to a different branch of the code"? Just a magic > number assumed to be reasonably small, or is there a hard boundary at 4kx4k > (1<<24)? There's a hard boundary, beyond that you lack the precision in the integral image fields. After all you can only store 2^24 times 8 bits in a 32-bit number :)
(In reply to Bas Schouten (:bas.schouten) from comment #20) > There's a hard boundary, beyond that you lack the precision in the integral > image fields. After all you can only store 2^24 times 8 bits in a 32-bit > number :) I have to admit mine was a leading question :-) If width is 4k, height is 4k, we go into else (as 4k*4k == 1<<24), and ask the AlignedArray to give us back 1<<24 + 12 (stride is 4*4k in this example) in AlphaBoxBlur::Blur(): AlignedArray<uint32_t> integralImage((integralImageStride / 4) * integralImageSize.height + 12); Is this OK?
Flags: needinfo?(bas)
The worst case scenario is width=1, height=16M. In the old code, 1*16M == 1<<24, so we would go into else, stride would be 16, we would end up asking for 64M+12, which is definitely > 1<<24.
Attachment #713509 - Flags: feedback?(bas)
(In reply to Milan Sreckovic [:milan] from comment #22) > Created attachment 713509 [details] [diff] [review] > Should we check for 2^24 given the alignment and padding instead? > > The worst case scenario is width=1, height=16M. In the old code, 1*16M == > 1<<24, so we would go into else, stride would be 16, we would end up asking > for 64M+12, which is definitely > 1<<24. Yes, this is padding we use for being able to read 16 bytes at a time for SSE2 reasons. As these do not result in additional accumulation that's okay.
Flags: needinfo?(bas)
Thanks. I was misled and confused by the code: if (something > 16M) { special case for large allocations } else { allocate 64M+ } and it isn't immediately obvious that this is what the author intended. Is there a good comment we can put in there to stop any further confusion? Or is it just confusing to me?
Dropping qawanted since attempts to reproduce this have proved to be fruitless. Please re-add qawanted if you have any new leads you'd like us to check.
Keywords: qawanted
(In reply to Milan Sreckovic [:milan] from comment #24) > Thanks. I was misled and confused by the code: > if (something > 16M) { > special case for large allocations > } else { > allocate 64M+ > } > and it isn't immediately obvious that this is what the author intended. Is > there a good comment we can put in there to stop any further confusion? Or > is it just confusing to me? I would argue the other (being me), did an extremely poor job at documenting this exception and a comment should definitely be added to clarify this! :)
Well 19 is out the door now and this wouldn't drive a chemspill, so wontfixing for 19.
Depends on: 845260
Bug 845260 has STR.
Bas, I don't understand what happened to this bug: this is an OOM crash. Are you saying that the math in (integralImageStride / 4) * integralImageSize.height + 12); is incorrect and we're asking for the wrong amount of memory? It seems that the obvious problem here is that the AlignedArray<uint32_t> is using an infallible allocator for large buffer allocation, and this should instead be using a fallible allocator and null-checking. See bug 857030 for somebody who experiences this. I expect that this crash is the primary cause for the increase of EMPTY DUMP crashes (bug 837835), which are our top stability priority right now.
Blocks: 837835, 834256
No longer depends on: 834256
Flags: needinfo?(bas)
Comment 0 shows mozilla::gfx::AlphaBoxBlur::Blur failing to allocate 1277804 bytes, which isn't really huge but should probably fail gracefully. Sounds like we might have two problems: 1) mozilla::gfx::AlphaBoxBlur::Blur can cause us to crash on OOM, could fail gracefully 2) If this memory doesn't get freed right away then this could cause us to OOM elsewhere, which might show up as an empty-dump OOM crash.
I think we should track this for FF21 given comment 29
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #31) > Comment 0 shows mozilla::gfx::AlphaBoxBlur::Blur failing to allocate 1277804 > bytes, which isn't really huge but should probably fail gracefully. Sounds > like we might have two problems: > 1) mozilla::gfx::AlphaBoxBlur::Blur can cause us to crash on OOM, could fail > gracefully > 2) If this memory doesn't get freed right away then this could cause us to > OOM elsewhere, which might show up as an empty-dump OOM crash. I'm not really sure how graceful we could fail? If we don't have room in our address space for 1277804 bytes doesn't that success we're done for and are better of crashing? How I'm not sure how using a fallible allocator and a null check would help? We'd just fail in some other part of (possibly GFX code) later shouldn't we? Wasn't the whole point to make out allocations infallible? Maybe I totally misunderstood.
Flags: needinfo?(bas)
Yeah, there's a fundamental misunderstanding of the infallible allocator here. The point of the infallible allocator was to make small and mostly fixed-size allocations fail. Large allocations (anything which has a content-controlled size) are still supposed to use fallible allocators, null-check, and fail more gracefully. This is especially important for huge allocations (>256k) which can fail do to VM page fragmentation even if there is enough available memory to handle the small allocations from normal browser operation.
Priority: -- → P1
Whiteboard: [MemShrink]
(In reply to Benjamin Smedberg [:bsmedberg] from comment #34) > Yeah, there's a fundamental misunderstanding of the infallible allocator > here. > > The point of the infallible allocator was to make small and mostly > fixed-size allocations fail. Large allocations (anything which has a > content-controlled size) are still supposed to use fallible allocators, > null-check, and fail more gracefully. This is especially important for huge > allocations (>256k) which can fail do to VM page fragmentation even if there > is enough available memory to handle the small allocations from normal > browser operation. At this point we know we're going to actively producing artifacts though, and it's going to be particularly interesting to have higher level code deal gracefully with surfaces not having been created and initialized properly. Making this allocation fallible won't be a big issue, but having that continue into a positive browsing experience is going to be a more interesting challenge.
That's true; but comment data shows that users on low-memory machines pay attention to things like rendering artifacts as a sign of low memory and they will quit/restart Firefox. I think it's worth it in this case at least.
Whiteboard: [MemShrink] → [MemShrink:P1]
Attached patch Make AlphaBoxBlur allocation fallible (obsolete) (deleted) — Splinter Review
Attachment #738291 - Flags: review?(jmuizelaar)
Attachment #738291 - Flags: review?(jmuizelaar) → review+
It has entered the top-20 crashers in 20.0.1.
Keywords: topcrash
Bas, do you know why both these bugs bounced?
Flags: needinfo?(bas)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #41) > Bas, do you know why both these bugs bounced? I hadn't noticed! Ugh. I'm on it.
Flags: needinfo?(bas)
Crash Signature: [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()] [@ mozalloc_abort(char const* const) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()] → [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()] [@ mozalloc_abort(char const* const) | moz_xmalloc | mozilla::gfx::AlphaBoxBlur::Blur()] [@ moz_abort | arena_run_split | arena_…
(In reply to Wayne Mery (:wsmwk) from comment #43) > me bp-d70e783f-4317-4ad4-a0e0-802a12130430 (not me, but dee, who has a variety of OOM crash sigs only one of which is AlphaBoxBlur) bp-6429ea66-8650-4b52-8b9a-09b8f2130510
Flags: needinfo?(bas)
Pinging about this in email.
Attachment #738291 - Attachment is obsolete: true
Flags: needinfo?(bas)
Comment on attachment 755862 [details] [diff] [review] Make AlignedArray use fallible allocation. Review of attachment 755862 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Tools.h @@ +109,5 @@ > > MOZ_ALWAYS_INLINE void Realloc(size_t aSize) > { > delete [] mStorage; > + mStorage = new (nothrow) T[aSize + (alignment - 1)]; Should be static fallible_t fallible = fallible_t(); mStorage = new (fallible) T[aSize + (alignment - 1)];
(In reply to :Ms2ger from comment #47) > Comment on attachment 755862 [details] [diff] [review] > Make AlignedArray use fallible allocation. > > Review of attachment 755862 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/2d/Tools.h > @@ +109,5 @@ > > > > MOZ_ALWAYS_INLINE void Realloc(size_t aSize) > > { > > delete [] mStorage; > > + mStorage = new (nothrow) T[aSize + (alignment - 1)]; > > Should be > > static fallible_t fallible = fallible_t(); > mStorage = new (fallible) T[aSize + (alignment - 1)]; Why would we do something so convoluted? Does jemalloc not provide the right overloads? And if so why wouldn't we do: mStorage = new (fallible_t()) T[aSize + (alignment - 1)]; At least that's a little less ugly. In any case, inside Moz2D we don't have the fallible_t() struct, so that's not going to happen anyway :).
Attachment #756315 - Flags: review?(jmuizelaar)
Attachment #755862 - Flags: review?(jmuizelaar)
We're going to build with b4 on Tuesday, so would be great to get review/landed prior.
Comment on attachment 756315 [details] [diff] [review] Part 2: Check AlignedArray for allocation success. Review of attachment 756315 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Blur.cpp @@ +469,5 @@ > + unsigned char* tmpData = new (std::nothrow) uint8_t[szB]; > + > + if (!tmpData) { > + return; > + } If this works it seems ok.
Attachment #756315 - Flags: review?(jmuizelaar) → review+
Attachment #755862 - Flags: review?(jmuizelaar) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
So, this landed on central, and on beta without explicit approval, shouldn't we get it into aurora as well?
(In reply to Robert Kaiser (:kairo@mozilla.com) [away until early June] from comment #54) > So, this landed on central, and on beta without explicit approval, shouldn't > we get it into aurora as well? Approval was given over email
And yes, we should uplift to Aurora.
Flags: needinfo?(bas)
Attachment #755862 - Flags: approval-mozilla-beta+
Attachment #755862 - Flags: approval-mozilla-aurora+
Attachment #756315 - Flags: approval-mozilla-beta+
Attachment #756315 - Flags: approval-mozilla-aurora+
Flags: needinfo?(bas)
No crashes on FF > 21. I guess we can call this bug verified.
Attachment #713509 - Flags: feedback?(bas)
Flags: needinfo?(amanda)
Flags: needinfo?(amanda)
Restrict Comments: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: