1578075 - [macOS 10.15] Content process crashes in CoreGraphics`crossing_count() due to stack overflow

Status: VERIFIED FIXED

(Core :: Graphics: Text, defect, P1)

Description

[Daniel]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

1. Go to https://stronghold.co/careers/
2. Wait

macOS 10.15 “Catalina” Beta (19A546d)

Actual results:

Crashes after zero–three seconds. Happens every time.
https://crash-stats.mozilla.org/report/index/01b19d74-15e2-4fa3-8ffd-e91690190901

Expected results:

The page should load.

Comment 1

[[:philipp]]

hi, the crash report you've mentioned is coming from catalina beta 5 still which had a general crashing problem. do you have a more recent crash report available?

Comment 2

[Simona Badau, Desktop QA]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:71.0) Gecko/20100101 Firefox/71.0
Build ID: 20190904163258
macOS Catalina - version 10.15 Beta 7 (19A546d)

I can confirm that Firefox crashes when loading the provided URL - tested with Firefox 69, Firefox 70 beta 3 and Nightly 71.0a1.
Here are some crashes reports:

• on Firefox 69: https://crash-stats.mozilla.org/report/index/610da4c6-2536-419d-a068-85a040190905
• on Firefox 70 beta 3: https://crash-stats.mozilla.org/report/index/c2fa22ed-4df9-41ee-8267-1e3620190905
• on Nightly 71.0a1: crash reports are not generated - please see Bug 1579048

This issue is not reproducible on Mac OS X 10.14.
This issues is reproducible also on the Mac 10.15 Beta 6(19A536g)

Comment 4

[Marcia Knous [:marcia]]

Until we get better symbols for 10.15, it may be more difficult to isolate this. The CoreGraphics crash signature was the one generated when users were crashing on 10.15 Beta 5, so I am removing that to make it clear that this crash is happening on 19A546d, which is the latest seed.

Comment 5

[Markus Stange [:mstange]]

Edit: The stack below is obsolete; it's the stack for a crash in an older Beta that has since been fixed by Apple and is not related to the tab crash that this bug was filed about.

Original comment below.

---

Symbolicated stack for https://crash-stats.mozilla.org/report/index/01b19d74-15e2-4fa3-8ffd-e91690190901:

#0 CGDataProviderCopyPixelData (in CoreGraphics)
#1 CGContextSynchronize (in CoreGraphics)
#2 CGSImageDataLockDevice (in CoreGraphics)
#3 SLSGetPerformanceTotalUpdateCount (in SkyLight)
#4 HIView::Validate(__HIShape const*) (in HIToolbox)
#5 HIView::GetFocusPartSelf() (in HIToolbox)
#6 HIMenuBarView::DrawOnce(CGRect, CGRect, bool, bool, CGContext*) (in HIToolbox)
#7 _HIThemeReduceTransparency (in HIToolbox)
#8 IsMenuBarTransparencyEnabled (in HIToolbox)
#9 DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) (in HIToolbox)
#10 _NotifyEventLoopObservers (in HIToolbox)
#11 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) (in HIToolbox)
#12 HIView::ShapeToViewShape(__HIShape const*, HIView*, __HIShape*) (in HIToolbox)
#13 HIView::RecursiveDrawComposited(__HIShape const*, __HIShape const*, unsigned int, HIView*, CGContext*, unsigned char, double) (in HIToolbox)
#14 HIView::SendDraw(short, OpaqueGrafPtr*, __HIShape const*, CGContext*) (in HIToolbox)
#15 HIView::BuildAggregateInvalidRegion(__HIShape*) (in HIToolbox)
#16 HIView::DrawComposited(short, OpaqueGrafPtr*, __HIShape const*, unsigned int, HIView*, CGContext*) (in HIToolbox)
#17 HIView::Invalidate(__HIShape const*, unsigned char) (in HIToolbox)
#18 ___ZL16GetFlushObserverv_block_invoke (in HIToolbox)
#19 UpdateLatentWindowVisibility(WindowData*, unsigned char) (in HIToolbox)
#20 HIWindowCreate (in HIToolbox)
#21 MBWindows::GetWindowOnDisplay(unsigned int, unsigned char) (in HIToolbox)
#22 MenuBarInstance::ForEachWindowDo(unsigned char, bool (OpaqueWindowPtr*, unsigned int) block_pointer) (in HIToolbox)
#23 MenuBarInstance::GetBoundsOnSpace() (in HIToolbox)
#24 MenuBarInstance::Show(MenuBarAnimationStyle, unsigned char, unsigned char, unsigned char, unsigned char) (in HIToolbox)
#25 RemoveKeyFocusHotKeys (in HIToolbox)
#26 ConvertEventRefToEventRecord (in HIToolbox)
#27 GetAppropriateNextTarget(EventTargetRec*, HandlerCallRec*, OpaqueEventRef*, unsigned char, DelegateInfo*) (in HIToolbox)
#28 KeyEventDeQueuedObserver (in HIToolbox)
#29 RunCurrentEventLoopInMode (in HIToolbox)
#30 ReceiveNextEventCommon (in HIToolbox)
#31 _DPSNextEvent (in AppKit)
#32 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (in AppKit)

Comment 6

[Markus Stange [:mstange]]

Edit: This comment was about a crash in an older Beta that has since been fixed by Apple and is not related to the tab crash that this bug was filed about.

Original comment below.

---

Haik, can you make sure this crash is communicated to Apple? I think we need a better solution in general to make sure Apple knows about crashes that happen in Apple code in Firefox. I'm a bit unhappy about how our crash reporter basically cuts that communication channel between users and Apple.

Comment 7

[Haik Aftandilian [:haik]]

(In reply to Markus Stange [:mstange] from comment #6)

Haik, can you make sure this crash is communicated to Apple? I think we need a better solution in general to make sure Apple knows about crashes that happen in Apple code in Firefox. I'm a bit unhappy about how our crash reporter basically cuts that communication channel between users and Apple.

I've reported it to our Apple contact. Could you file a bug for the crash reporter issue? Maybe our crash reporter could dump crash info to the appropriate place and trigger Apple's crash reporter dialog. It seems like we might be able to do something better here.

Edit: Per Markus, the stack from comment 6 and comment 7 were not related to this bug. They were for a crash already fixed in Catalina.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

I see this too. Note that it's the tab loading the URL from comment #0 that crashes, not the entire browser.

I have the current Catalina Beta -- Beta 7 (build 19A546d, like the reporter).

I'll take out HookCase (https://github.com/steven-michaud/HookCase) for a whirl, and see what I can find.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Oops, spoke too soon. HookCase doesn't currently support Catalina (macOS 10.15), and I don't currently plan to add support for it until it's actually released (no longer in beta). Adding support for a new major version of macOS is usually a fairly large undertaking -- a couple of weeks' work.

Comment 10

[Markus Stange [:mstange]]

I can reproduce these tab crashes but Firefox does not create a crash report for them. Furthermore, the crash report from comment 0 (bp-01b19d74-15e2-4fa3-8ffd-e91690190901) looks very much like a parent process crash. So I think that report, and the stack I pasted in comment 5, are not from the URL in comment 0. Indeed, the report is from macOS version 10.15 19A526h and not from 19A546d.

Comment 11

[Markus Stange [:mstange]]

Oh, and :philipp already noticed the discrepancy in comment 1.

Comment 12

[Markus Stange [:mstange]]

Steven, even without HookCase, could you get a crash stack from the tab crash? The easiest way would probably be to do the following:

1. Open a tab and go to any web page that's not the new tab page, e.g. cnn.com.
2. Get the pid of the content process for that tab from the tab's tooltip.
3. Attach lldb to that process.
4. Navigate to the crashing URL, in the same tab.

I don't have a 10.15 development setup at the moment and regular Nightly builds are not debuggable on 10.15 (due to notarization), and I'm going on PTO tomorrow, otherwise I would try to get the stack myself.

Comment 13

[Haik Aftandilian [:haik]]

This was collected with a local debug build. Note the hundreds of omitted CoreGraphics`crossing_count() frames.

Process 44244 stopped
* thread #35, name = 'PaintWorker #3', stop reason = EXC_BAD_ACCESS (code=2, address=0x700009b2aff8)
    frame #0 CoreGraphics`crossing_count() 
CoreGraphics`crossing_count:
->  0x7fff375482a5 <+482>: callq  0x7fff3754858b            ; get_y_inflections_2
    0x7fff375482aa <+487>: jmp    0x7fff375482b4            ; <+497>
    0x7fff375482ac <+489>: vzeroupper 
    0x7fff375482af <+492>: callq  0x7fff375c37c9            ; get_y_inflections_3
Target 0: (plugin-container) stopped.
(lldb) bt
* thread #35, name = 'PaintWorker #3', stop reason = EXC_BAD_ACCESS (code=2, address=0x700009b2aff8)
  * frame #0 CoreGraphics`crossing_count() 
    frame #1 CoreGraphics`crossing_count() 
    frame #2 CoreGraphics`crossing_count() 
    frame #3 CoreGraphics`crossing_count() 
    frame #4 CoreGraphics`crossing_count() 
    frame #5 CoreGraphics`crossing_count() 
    frame #6 CoreGraphics`crossing_count() 
    frame #7 CoreGraphics`crossing_count() 
    frame #8 CoreGraphics`crossing_count() 
  ...
  <STACK FRAMES OMITTED>
  ...
    frame #848 CoreGraphics`crossing_count() 
    frame #849 CoreGraphics`crossing_count() 
    frame #850 CoreGraphics`crossing_count() 
    frame #851 CoreGraphics`crossing_count() 
    frame #852 CoreGraphics`crossing_count() 
    frame #853 CoreGraphics`crossing_count() 
    frame #854 CoreGraphics`crossing_count() 
    frame #855 CoreGraphics`crossing_count() 
    frame #856 CoreGraphics`crossing_count() 
    frame #857 CoreGraphics`crossing_count() 
    frame #858 CoreGraphics`crossing_count() 
    frame #859 CoreGraphics`path_evaluate_levels() 
    frame #860 CoreGraphics`path_get_expected_outside_orientation() 
    frame #861 CoreGraphics`path_fix_orientation() 
    frame #862 CoreGraphics`CGPathCreateByNormalizingGlyphPath() 
    frame #863 libFontParser.dylib`FPFontCopyGlyphPath() 
    frame #864 CoreGraphics`CGFontCreateGlyphPath() 
    frame #865 CoreGraphics`CGFontCreateGlyphBitmapWithDilation() 
    frame #866 CoreGraphics`CGGlyphBuilder::create_missing_bitmaps() 
    frame #867 CoreGraphics`CGGlyphBuilder::lock_glyph_bitmaps() 
    frame #868 CoreGraphics`render_glyphs() 
    frame #869 CoreGraphics`draw_glyph_bitmaps() 
    frame #870 CoreGraphics`ripc_DrawGlyphs() 
    frame #871 CoreGraphics`CGContextDelegateDrawGlyphs() 
    frame #872 CoreGraphics`draw_glyphs() 
    frame #873 CoreText`DrawGlyphsAtPositions() 
    frame #874 CoreText`CTFontDrawGlyphs() 
    frame #875 XUL`Offscreen::getCG() at SkFontHost_mac.cpp:1190
    frame #876 XUL`SkScalerContext_Mac::generateImage() at SkFontHost_mac.cpp:1406
    frame #877 XUL`SkScalerContext::getImage() at SkScalerContext.cpp:521
    frame #878 XUL`SkStrike::findImage() at SkStrike.cpp:131
    frame #879 XUL`SkGlyphRunListPainter::drawForBitmapDevice() at SkGlyphRunPainter.cpp:244
    frame #880 XUL`SkDraw::drawGlyphRunList() at SkDraw_text.cpp:129
    frame #881 XUL`SkBitmapDevice::drawGlyphRunList() at SkBitmapDevice.cpp:541
    frame #882 XUL`SkGlyphRunBuilder::drawTextBlob() at SkGlyphRun.cpp:232
    frame #883 XUL`SkCanvas::onDrawTextBlob() at SkCanvas.cpp:2552
    frame #884 XUL`SkCanvas::drawTextBlob() at SkCanvas.cpp:2573
    frame #885 XUL`SkCanvas::drawTextBlob() at SkCanvas.h:1983
    frame #886 XUL`mozilla::gfx::DrawTargetSkia::DrawGlyphs() at DrawTargetSkia.cpp:1389
    frame #887 XUL`mozilla::gfx::DrawTargetSkia::FillGlyphs() at DrawTargetSkia.cpp:1396
    frame #888 XUL`mozilla::gfx::FillGlyphsCommand::ExecuteOnDT() at DrawCommands.h:577
    frame #889 XUL`mozilla::gfx::DrawTargetCaptureImpl::ReplayToDrawTarget() at DrawTargetCapture.cpp:330
    frame #890 XUL`mozilla::gfx::DrawTarget::DrawCapturedDT() at DrawTarget.cpp:168
    frame #891 XUL`mozilla::layers::PaintThread::AsyncPaintTask() at PaintThread.cpp:206
    frame #892 XUL`mozilla::layers::PaintThread::QueuePaintTask(mozilla::UniquePtr<mozilla::layers::PaintTask, mozilla::DefaultDelete<mozilla::layers::PaintTask> >&&)::$_7::operator()() const() at PaintThread.cpp:178
    frame #893 XUL`mozilla::detail::RunnableFunction<mozilla::layers::PaintThread::QueuePaintTask(mozilla::UniquePtr<mozilla::layers::PaintTask, mozilla::DefaultDelete<mozilla::layers::PaintTask> >&&)::$_7>::Run() at nsThreadUtils.h:564
    frame #894 XUL`nsThreadPool::Run() at nsThreadPool.cpp:246
    frame #895 XUL`nsThread::ProcessNextEvent() at nsThread.cpp:1225
    frame #896 XUL`NS_ProcessNextEvent() at nsThreadUtils.cpp:486
    frame #897 XUL`mozilla::ipc::MessagePumpForNonMainThreads::Run() at MessagePump.cpp:333
    frame #898 XUL`MessageLoop::RunInternal() at message_loop.cc:315
    frame #899 XUL`MessageLoop::RunHandler() at message_loop.cc:308
    frame #900 XUL`MessageLoop::Run() at message_loop.cc:290
    frame #901 XUL`nsThread::ThreadFunc() at nsThread.cpp:458
    frame #902 libnss3.dylib`_pt_root() at ptthread.c:198
    frame #903 libsystem_pthread.dylib`_pthread_start() 
    frame #904 libsystem_pthread.dylib`thread_start() 
(lldb) 

Comment 14

[Steven Michaud [:smichaud] (Retired)]

It looks like Haik has already done what was requested. Seems like a stack overflow caused by infinite recursion.

Comment 15

[Haik Aftandilian [:haik]]

I ran across bug 1471892 "Crash in CoreGraphics@0x26fc8 stack overflow in crossing_count" where the workaround was to set layers.omtp.enabled to false. That also worked for this crash in my testing.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

I just reproduced this crash again, as a tab crash. Here's my crash report:

https://crash-stats.mozilla.org/report/index/284b072c-ba13-4eef-a9db-339520190909

Seems very different from what Haik reported. But that kind of thing is consistent with an infinite recursion.

Comment 17

[Lee Salzman [:lsalzman]]

(In reply to Haik Aftandilian [:haik] from comment #15)

I ran across bug 1471892 "Crash in CoreGraphics@0x26fc8 stack overflow in crossing_count" where the workaround was to set layers.omtp.enabled to false. That also worked for this crash in my testing.

That bug implies that the stack size is just to small for our paint threads, and that is the root of the problem. Though in that bug they made the call to just disable OMTP on 10.9. It would be a bummer to have to disable OMTP on 10.15, though. Maybe we just want to increase the stack size on OMTP threads? Would it really be that bad to chew up a couple more MB per process for OMTP?

Comment 18

[Steven Michaud [:smichaud] (Retired)]

But if this bug really is an infinite recursion, wouldn't the stack always get eaten up, no matter what size it is?

Comment 19

[Haik Aftandilian [:haik]]

(In reply to Lee Salzman [:lsalzman] from comment #17)

(In reply to Haik Aftandilian [:haik] from comment #15)

I ran across bug 1471892 "Crash in CoreGraphics@0x26fc8 stack overflow in crossing_count" where the workaround was to set layers.omtp.enabled to false. That also worked for this crash in my testing.

That bug implies that the stack size is just to small for our paint threads, and that is the root of the problem. Though in that bug they made the call to just disable OMTP on 10.9. It would be a bummer to have to disable OMTP on 10.15, though. Maybe we just want to increase the stack size on OMTP threads? Would it really be that bad to chew up a couple more MB per process for OMTP?

Increasing the stack size for 10.15 sounds like a good approach (assuming this is not reproducible on 10.14 and earlier). On 10.15, I tested a quick hack/fix to double the stack size in PaintThread::InitPaintWorkers() from 256k to 512k and can not reproduce the problem with that change. Someone from graphics who is more knowledgeable about off-main-thread-painting should really take this bug and determine the right upper limit though.

Comment 20

[Markus Stange [:mstange]]

I don't think anybody will have a good idea of what the right upper limit is, because I don't think we have a way to estimate the recursion depth. We should increase it to a value that works until we find more crashes. 512k sounds fine to me.

Comment 21

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #18)

This is almost certainly an Apple bug. So if we can "fix" it by disabling OMTP, that need be only until either Apple fixes the real bug or we figure out a more comfortable workaround.

Comment 22

[Haik Aftandilian [:haik]]

(In reply to Steven Michaud [:smichaud] (Retired) from comment #18)

But if this bug really is an infinite recursion, wouldn't the stack always get eaten up, no matter what size it is?

(In reply to Steven Michaud [:smichaud] (Retired) from comment #21)

(Following up comment #18)

This is almost certainly an Apple bug. So if we can "fix" it by disabling OMTP, that need be only until either Apple fixes the real bug or we figure out a more comfortable workaround.

Steven, from my very limited graphics knowledge, "crossing count" is a recursive path rendering algorithm and the recursion limit may be content-dependent, but were you thinking this much recursion should be considered a bug? Given that a larger stack size avoided the crash, it's not an infinite recursion and Lee's response on comment 17 implied some recursion was expected.

Comment 23

[Steven Michaud [:smichaud] (Retired)]

(In reply to Haik Aftandilian [:haik] from comment #22)

Steven, from my very limited graphics knowledge, "crossing count" is a recursive path rendering algorithm and the recursion limit may be content-dependent, but were you thinking this much recursion should be considered a bug? Given that a larger stack size avoided the crash, it's not an infinite recursion and Lee's response on comment 17 implied some recursion was expected.

If increasing the stack size can "fix" this bug, then it isn't an infinite recursion (just a large one). So I stand corrected. And if Firefox has always been using "too small" a stack, it may not even be an Apple bug. Lacking documentation on how large the stack should be, we can only find out by looking at apps that we know Apple tests a lot -- like Safari. I wouldn't be surprised to find that the minimal stack size has somehow increased on macOS 10.15.

Comment 24

[Jeff Muizelaar [:jrmuizel]]

FWIW, our paint threads have less stack then the default main thread size. I suspect CoreGraphics is mostly tested on threads that have at least the default main thread size.

Comment 25

[Jeff Muizelaar [:jrmuizel]]

Also, it would be useful if someone could figure out what the problematic font was.

Comment 26

[Haik Aftandilian [:haik]]

[Tracking Requested - why for this release]:
ESR 60 is scheduled to EOL on October 23rd and macOS 10.15 Catalina is likely to release before that time.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Romain, we don't have any further planned ESR60 releases prior to EOL. Is this worth spinning a dot release over or can we just accept that given the relatively short window of time ESR60 will be officially supported for after 10.15 is released?

Comment 28

[Ryan Hunt [:rhunt]]

I agree with Lee that the best option is to increase the stack size on Catalina. I've got a patch that does this on try.

Comment 29

[Ryan Hunt [:rhunt]]

I don't have Catalina currently. Would someone be willing to try the attached build and see if this resolves the issue? [1] [2]

I just realized this is a debug build, not sure if that would significantly affect sizes of things on the stack. Spinning up a release build here. [3]

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=02de63b646fa0ade752c91bc91f052f81518e1e8
[2] https://queue.taskcluster.net/v1/task/CAtpjjGTTPGxCwj1iuGYdg/runs/0/artifacts/public/build/target.dmg
[3] https://treeherder.mozilla.org/#/jobs?repo=try&revision=075551681c10a974eb3202a3ca4d563350b85829

Comment 30

[Haik Aftandilian [:haik]]

I'll test the opt build later today when its available and report back.

Comment 31

[Steven Michaud [:smichaud] (Retired)]

I just tried the debug build from comment #29 (https://queue.taskcluster.net/v1/task/CAtpjjGTTPGxCwj1iuGYdg/runs/0/artifacts/public/build/target.dmg), and didn't crash. I visited the URL from comment #0.

Comment 32

[Haik Aftandilian [:haik]]

I tested the opt build and couldn't reproduce the crash.

@Ryan, thanks for the quick fix. So we have an idea of the change in memory use, it looks like the paint worker thread pool will have at most 4 threads. Is there just one pool per content process so this will be at most an extra 1MB (4 threads * 256K stack increase)?

Comment 33

[Ryan Hunt [:rhunt]]

Thanks for confirming!

Yes, the paint thread will have up to 4 workers leading to a potential 1MB increase in memory when the stack is fully used.

Comment 34

[Ryan Hunt [:rhunt]]

Attachment: Bug 1578075 - Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r?jrmuizel

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd0ea6dafa05
Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r=jrmuizel

Comment 36

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/cd0ea6dafa05

Comment 37

[Romain Testard [:RT]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #27)

Romain, we don't have any further planned ESR60 releases prior to EOL. Is this worth spinning a dot release over or can we just accept that given the relatively short window of time ESR60 will be officially supported for after 10.15 is released?

No planned ESR60 releases prior to EOL I am aware of.
I suspect the intercept between Catalina and ESR60 users will be small inside Enterprises but it's hard to know for sure given how little telemetry we get from ESR.
I feel like the pain here is not worth the value but NIing mkaply in case he thinks differently.

Comment 38

[Mike Kaply [:mkaply]]

Yeah, it's going to be close with Catalina/ESR. Do we know if Apple is going to do anything in Catalina to remedy this? Or is this definitely our bug.

My gut says that if someone is on ESR, they probably aren't going to Catalina quickly.

Comment 39

[Georg Koppen]

(In reply to Mike Kaply [:mkaply] from comment #38)

Yeah, it's going to be close with Catalina/ESR. Do we know if Apple is going to do anything in Catalina to remedy this? Or is this definitely our bug.

My gut says that if someone is on ESR, they probably aren't going to Catalina quickly.

Well, speaking out of experience with the whole Catalina beta cycle: we had surprisingly many bug reports for Tor Browser (which is still on ESR 60 until EOL) by users that were already running the beta versions of Catalina. And given that I assume that Apple will prominently show macOS users a "Hey, here is your new OS version, please update" I suspect that quite a bunch of our macOS users will be affected by this bug.

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Does Tor browser need us to do an out-of-band release to address this bug for their users?

Comment 41

[Georg Koppen]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #40)

Does Tor browser need us to do an out-of-band release to address this bug for their users?

I don't think so. However, we'll very likely need to do a release for that issue. So, having help with an ESR60 patch would be really neat given that we don't have the necessary macOS experience (and assuming the patch is not applying cleanly and not is not trivial to rebase, which I have not checked yet).

Comment 42

[Ryan VanderMeulen [:RyanVM]]

Please request Beta/Release/ESR68 approval on this when you get a chance. It grafts cleanly to all 3. If we did want to do something for ESR60, it'll need a rebased patch, however.

Comment 43

[Ryan Hunt [:rhunt]]

Comment on attachment 9091767 [details]
Bug 1578075 - Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Content process crash when visiting pages with certain fonts on OSX Catalina.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Quoting comment #0

Steps to reproduce:

    Go to https://stronghold.co/careers/
    Wait

macOS 10.15 “Catalina” Beta (19A546d)

Actual results:

Crashes after zero–three seconds. Happens every time.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We workaround this crash by increasing the amount of stack space we use on paint threads on OSX Catalina. This increases memory usage, but shouldn't functionally change much, it's limited to painting code.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a content process crasher that seems likely to be triggered easily when OSX Catalina is released.
• User impact if declined: Crashes when browsing to certain pages.
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We workaround this crash by increasing the amount of stack space we use on paint threads on OSX Catalina. This increases memory usage, but shouldn't functionally change much, it's limited to painting code.
• String or UUID changes made by this patch:

Comment 44

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9091767 [details]
Bug 1578075 - Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r?jrmuizel

Approved for 70.0b6 so we can get some wider feedback before the 69.0.1 dot release build next week.

Comment 45

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ffefc518e569

Comment 46

[ovidiu boca[:Ovidiu]]

I verified this issue on Nightly 71.0a1(2019-09-12) on Mac OS X 10.15 “Catalina” Beta (19A546d) and I can confirm the fix.

Comment 47

[ovidiu boca[:Ovidiu]]

I verified this issue on Beta 70.0b6 on Mac OS X 10.15 “Catalina” Beta (19A546d) and I can confirm the fix.

Comment 48

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9091767 [details]
Bug 1578075 - Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r?jrmuizel

Fixes a macOS 10.15 crash. Approved for 69.0.1.

Comment 49

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/7f7ce2c5764f

Comment 50

[Simona Badau, Desktop QA]

Verified as fixed also on Firefox 69.0.1 on Mac OS X 10.15 “Catalina” Beta (19A558d).

Comment 51

[Ryan VanderMeulen [:RyanVM]]

Looks like we're still seeing some crashes with this signature in builds containing this fix :(

Comment 52

[Ryan Hunt [:rhunt]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #51)

Looks like we're still seeing some crashes with this signature in builds containing this fix :(

That's unfortunate..

I think our only option is to try to increase the reserved stack size further. OSX has a default stack size of 8MB for the main thread and we're currently at 512kb with the patch that landed.

Do we happen to have any links to pages involved in these crashes? It would be convenient to have some idea of how much further we need to increase the stack size by. Otherwise maybe we increase to 2MB?

Comment 53

[Ryan VanderMeulen [:RyanVM]]

Nothing immediately obvious in the reports I've been looking through (about:sessionstore, about:home, etc). There were a few reports with email addresses included - I'll try reaching out to them to see if there's any leads there.

Comment 54

[Ryan VanderMeulen [:RyanVM]]

I heard back from one of the people I emailed but wasn't able to get any more actionable information. Should we try bumping the thread size higher and just see how it goes?

Comment 55

[Jessie [:jbonisteel] pls NI]

Ryan pinged me, he is away this week

Lee would you be able to try bumping up the thread size?

Comment 56

[Lee Salzman [:lsalzman]]

Attachment: Bug 1578075 - Increase stack size of paint threads on macOS Catalina to 1MB. r?jrmuizel

Comment 57

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d2de2fa28ed
Increase stack size of paint threads on macOS Catalina to 1MB. r=jrmuizel

Comment 58

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/1d2de2fa28ed

Comment 59

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance. I still see a couple new reports on Nightly, but it may be worth still seeing how this does on a wider audience.

Comment 60

[Lee Salzman [:lsalzman]]

Comment on attachment 9094698 [details]
Bug 1578075 - Increase stack size of paint threads on macOS Catalina to 1MB. r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Want to see if this decreases the crashes to a reasonable level on beta...
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 61

[Lee Salzman [:lsalzman]]

Attachment: Bug 1578075 - Increase stack size of paint threads on macOS Catalina to 2MB. r?jrmuizel

Still getting some crash reports on nightly with a 1MB stack. Let's try one more time
with 2MB just to see if we can eliminate those. If this fails, then we may need to consider
more drastic approaches like disabling OMTP in this case.

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bede638ee395
Increase stack size of paint threads on macOS Catalina to 2MB. r=jrmuizel

Comment 63

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/bede638ee395

Comment 64

[Lee Salzman [:lsalzman]]

I discussed with Ryan that based on the evidence I think there are two different crash signatures getting confused here. The original report doesn't show signs of a stack overflow, and instead is a nullpointer deref (CoreGraphics@0x197084) .

Whereas the other reports (@0x7fff36dea2a5) indicate a stack overflow inside the crossing_count() function, crashing on a non-null address, which based on the crash reports I think were fixed by Ryan's original patch, and did not require my two patches after the fact. The CoreGraphics@ signature did not respond to my bumps in stack size either, so I believe them to just be unrelated and advise that we just backout my patches.

We should probably split off the CoreGraphics@ signature after backing out my patches, then see if we can investigate why that signature is being generated independent from the fix we did in this bug thread for the stack overflow.

Comment 65

[Ryan VanderMeulen [:RyanVM]]

We haven't seen any reports of the @0x7fff36dea2a5 signature in builds with Ryan's original fix, so that sounds reasonable to me.

Comment 66

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1578075 - Revert stack size of paint threads back to 512KB on macOS 10.15. r=lsalzman

Comment 67

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9091767 [details]
Bug 1578075 - Increase stack size of paint thread/workers on OSX Catalina or higher to workaround crash from recursion in CoreText. r?jrmuizel

Approved for 68.2esr.

Comment 68

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/931d0d1bacc1
Revert stack size of paint threads back to 512KB on macOS 10.15. r=lsalzman

Comment 69

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/931d0d1bacc1

Comment 70

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/616598697716

Comment 71

[ovidiu boca[:Ovidiu]]

I verified this issue on Mac OS X 10.15 with FF ESR 68.2.0 and I can confirm the fix.

Comment 72

[Steven Michaud [:smichaud] (Retired)]

Oops, changed these by accident.

Comment 73

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This bug has been identified as part of a pilot on determining root causes of blocking and dot release drivers.

It needs a root-cause set for it. Please see the list at https://docs.google.com/document/d/1FFEGsmoU8T0N8R9kk-MXWptOPtXXXRRIe4vQo3_HgMw/.

Add the root cause as a whiteboard tag in the form [rca - <cause> ] and remove the rca-needed keyword.

If you have questions, please contact :tmaity.