1374888 - Crash in SharedLibraryInfo::GetInfoForSelf

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Ting-Yu Chou [:ting] (away)]

This bug was filed from the Socorro interface and is 
report bp-fa9750e8-11a2-4f98-9003-f2d640170619.
=============================================================
Top #2 of Nightly 20170618030207 on Mac OSX, 8 reports from 8 installations.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Just ran into this:

https://crash-stats.mozilla.com/report/index/ac97bb1b-21ce-4d58-b582-2e72f0170621

Comment 2

[Andrew McCreight [:mccr8]]

40 crashes on Nightly in the last 7 days, which is many more than other branches.

In the half dozen I looked at, this was mostly in mozilla::Telemetry::GetStackAndModules being called by the BHR. Possible regression from bug 1369594, which moved this off the main thread? Georg, do you know who might be able to look at this?

Comment 3

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: OSX crash that has gotten a lot worse in 56.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

This looks a lot like bug 1373980. Do we have any crashes on revisions after I landed that fix?

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(In reply to Michael Layzell [:mystor] from comment #4)
> This looks a lot like bug 1373980. Do we have any crashes on revisions after
> I landed that fix?

(I just checked. Yes we do - that didn't fix the problem apparently)

Comment 6

[Georg Fritzsche [:gfritzsche]]

I think Michael probably has the best context here.

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Maintain a live shared libraries list in gecko on macOS

I hope this should help us avoid some of the race problems with dyld unloading or loading libraries while we're trying to read their headers.

MozReview-Commit-ID: 5QvrWujquIq

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8884966 [details] [diff] [review]
Maintain a live shared libraries list in gecko on macOS

Review of attachment 8884966 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't look bad at all!

It basically spells doom for my patch in bug 1362277, but other than that, it seems fine.

::: tools/profiler/core/shared-libraries-macos.cc
@@ +41,5 @@
> +{
> +  const platform_mach_header* header;
> +  nsCString path;
> +};
> +static nsTArray<NativeSharedLibrary> sSharedLibrariesList;

Does nsTArray cause static initializers?

@@ +155,4 @@
>    SharedLibraryInfo sharedLibraryInfo;
>  
> +  for (auto& info : sSharedLibrariesList) {
> +    addSharedLibrary(info.header, info.path.get(), sharedLibraryInfo);

This .get() and the nsDependentCString(·) in addSharedLibrary cancel each other out. Please adjust the parameter type of addSharedLibrary.

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Maintain a live shared libraries list in gecko on macOS

Updated version which avoids destroying strings in the static destructor by doing some ugly dance with UniquePtrs.

MozReview-Commit-ID: 5QvrWujquIq

Comment 10

[Markus Stange [:mstange]]

Comment on attachment 8885369 [details] [diff] [review]
Maintain a live shared libraries list in gecko on macOS

Review of attachment 8885369 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/profiler/core/shared-libraries-macos.cc
@@ +61,5 @@
> +
> +  StaticMutexAutoLock lock(sSharedLibrariesMutex);
> +  if (!sSharedLibrariesList.get()) {
> +    sSharedLibrariesList = MakeUnique<nsTArray<NativeSharedLibrary>>();
> +    ClearOnShutdown(&sSharedLibrariesList);

I would do the initialization of sSharedLibrariesList in SharedLibraryInfo::Initialize() and then bail out here like you do in RemoveImage, just to cover the (hopefully extremely rare) case where SharedLibraryAddImage is called late during shutdown if ClearOnShutdown has already destroyed sSharedLibrariesList.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Maintain a live shared libraries list in gecko on macOS

MozReview-Commit-ID: 5QvrWujquIq

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b297e31320fd
Maintain a live shared libraries list in gecko on macOS, r=mstange

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for mass-asserting, e.g. in dom/base/test/test_setInterval_from_start.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c50a2e45d303a64d805485a2dc0eab1bef20c5e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b297e31320fde94696e2d5ae6d24369fa6b3caed&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=113410882&repo=mozilla-inbound

11:41:13     INFO - GECKO(1910) | [Parent 1910] WARNING: '!aObserver', file /home/worker/workspace/build/src/xpcom/ds/nsObserverService.cpp, line 243
11:41:13     INFO - GECKO(1910) | [NPAPI 1915] WARNING: '!compMgr', file /home/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp, line 52
11:41:13     INFO - GECKO(1910) | [NPAPI 1915] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /home/worker/workspace/build/src/xpcom/base/nsTraceRefcnt.cpp, line 171
11:41:13     INFO - GECKO(1910) | Assertion failure: 0 == rv, at /home/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:292
11:41:13     INFO - GECKO(1910) | Redirecting call to abort() to mozalloc_abort
11:41:13     INFO - GECKO(1910) | Hit MOZ_CRASH() at /home/worker/workspace/build/src/memory/mozalloc/mozalloc_abort.cpp:33

Comment 14

[Nika Layzell [:nika] (ni? for response)]

Attachment: Maintain a live shared libraries list in gecko on macOS

I switched it to using std::vector and std::string so we can dodge the leak checking, as we don't actually care about leaking here.

Re-requesting review.

MozReview-Commit-ID: 5QvrWujquIq

Comment 15

[Markus Stange [:mstange]]

Comment on attachment 8885804 [details] [diff] [review]
Maintain a live shared libraries list in gecko on macOS

Review of attachment 8885804 [details] [diff] [review]:
-----------------------------------------------------------------

But we still care about static initializers, don't we? I think we need a hybrid solution with UniquePtr<std::vector<...>>.

Comment 16

[Nika Layzell [:nika] (ni? for response)]

Attachment: Maintain a live shared libraries list in gecko on macOS

I changed it to allocate and leak the allocation. Not sure if this will throw off the leak detectors but I hope not. I don't think we run asan on macos so I think it should be fine?

MozReview-Commit-ID: 5QvrWujquIq

Comment 17

[Markus Stange [:mstange]]

Comment on attachment 8885811 [details] [diff] [review]
Maintain a live shared libraries list in gecko on macOS

Review of attachment 8885811 [details] [diff] [review]:
-----------------------------------------------------------------

We can change it if these leaks start throwing our tools off.

Comment 18

[Andrew McCreight [:mccr8]]

(In reply to Michael Layzell [:mystor] from comment #16)
> I changed it to allocate and leak the allocation. Not sure if this will
> throw off the leak detectors but I hope not. I don't think we run asan on
> macos so I think it should be fine?

You are right, we don't ASan on MacOS. However, in this case it wouldn't matter, because ASan doesn't treat things reachable from global variables as leaks. Your only concern would be the XPCOM leak checker, but it doesn't look like you are allocating anything the XPCOM leak checker tracks.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4bd519bfbfa
Maintain a live shared libraries list in gecko on macOS, r=mstange

Comment 20

[Carsten Book [:Tomcat]]

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