Closed Bug 1526216 Opened Last year Closed 11 months ago

Crash in nsFontFaceLoader::Cancel() in ASAN builds

Categories

(Core :: Layout: Text and Fonts, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 --- fixed
firefox67 --- fixed

People

(Reporter: hanno, Assigned: emilio)

References

Details

(Keywords: regression)

Attachments

(1 file)

I'm getting regular crashes in the ASAN build in nsFontFaceLoader::Cancel().

Unfortunately I'm unable to reliably reproduce those, but they happen when trying to close Firefox during a page load.

There's a bug with a similar looking stack trace in #1523181, but that's supposedly fixed and the line numbers don't match. (I updated the asan build this morning and still see the crash, so the fix for #1523181 should be in.)

Stack trace:

==18537==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f9387355523 bp 0x7ffeb063af10 sp 0x7ffeb063aef0 T0)
==18537==The signal is caused by a READ memory access.
==18537==Hint: address points to the zero page.
#0 0x7f9387355522 in nsFontFaceLoader::Cancel() /builds/worker/workspace/build/src/layout/style/nsFontFaceLoader.cpp:333:12
#1 0x7f938727cb6d in mozilla::dom::FontFaceSet::Disconnect() /builds/worker/workspace/build/src/layout/style/FontFaceSet.cpp:193:25
#2 0x7f938727c3cd in mozilla::dom::FontFaceSet::cycleCollection::Unlink(void*) /builds/worker/workspace/build/src/layout/style/FontFaceSet.cpp:89:8
#3 0x7f937eb87b05 in nsCycleCollector::CollectWhite() /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3084:26
#4 0x7f937eb8a8a1 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3430:24
#5 0x7f937eb8a474 in nsCycleCollector::ShutdownCollect() /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3351:10
#6 0x7f937eb8ee45 in Shutdown /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3639:5
#7 0x7f937eb8ee45 in nsCycleCollector_shutdown(bool) /builds/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:3992
#8 0x7f937ed7e911 in mozilla::ShutdownXPCOM(nsIServiceManager*) /builds/worker/workspace/build/src/xpcom/build/XPCOMInit.cpp:735:3
#9 0x7f938b15f75c in XRE_TermEmbedding() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:213:3
#10 0x7f937fce6d02 in mozilla::ipc::ScopedXREEmbed::Stop() /builds/worker/workspace/build/src/ipc/glue/ScopedXREEmbed.cpp:90:5
#11 0x7f938b1601d3 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:750:16
#12 0x558bed51b3d4 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:49:28
#13 0x558bed51b3d4 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:265
#14 0x7f9399b0109a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
#15 0x558bed440aa8 in _start (/home/osboxes/ff/firefox/firefox+0x2aaa8)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/layout/style/nsFontFaceLoader.cpp:333:12 in nsFontFaceLoader::Cancel()
==18537==ABORTING

This looks a lot like bug 1523181 which was fixed only 3 days ago. Can you confirm that your build is newer than that? We also have a few more open bugs around nsFontFaceLoader right now, so might also be one of those.

Reporter says "I updated the asan build this morning and still see the crash", so the fix from bug 1523181 ought to be there. Bug 1524246 also looks possibly related.

Emilio, can you have a look? Seems like this could be similar to issues you've been fixing, or even possibly a regression from one of the recent patches.

Flags: needinfo?(emilio)
Priority: -- → P2

Yes, I'll have a look, thanks. Yeah, looks like this is after bug 1524246 landed.

Do you have a crash report I could look at?

Valentin, looks like the previous code was calling nsIChannel::Cancel after OnStopRequest has been called. Is it ok not to do that? If so we just need a null-check. Otherwise we need to keep the channel around for longer.

Flags: needinfo?(emilio) → needinfo?(valentin.gosu)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

Yes, I'll have a look, thanks. Yeah, looks like this is after bug 1524246 landed.

Do you have a crash report I could look at?

Valentin, looks like the previous code was calling nsIChannel::Cancel after OnStopRequest has been called. Is it ok not to do that? If so we just need a null-check. Otherwise we need to keep the channel around for longer.

I don't think there's any restriction on calling Cancel after OnStopRequest - none that I can see in nsIRequest.idl or in the HttpChannel implementation. If it is called after OnStopRequest it shouldn't have any effect.

Flags: needinfo?(valentin.gosu)

Ok, null-checking it should be harmless then.

If there's no channel the request is over and the Cancel() call would be a no-op
anyway.

Assignee: nobody → emilio
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/724ed6f24523
Add a null-check for mChannel in nsFontFaceLoader::Cancel. r=heycam
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Please nominate this for Beta approval when you get a chance.

Blocks: 1519918
Flags: needinfo?(emilio)

The regressing patch was backed out from beta in bug 1526252

Flags: needinfo?(emilio)
You need to log in before you can comment on or make changes to this bug.