Prefixes before mozilla::detail::MutexImpl::mutexLock are platform-specific and do not add information. If they were stripped, it would be much easier to identify cross-platform issues. This is particularly striking on Linux, where the prefix is not symbolized: https://crash-stats.mozilla.org/search/?signature=%40libc.%2AmutexLock.%2A&date=%3E%3D2020-10-21T09%3A06%3A00.000Z&date=%3C2021-01-21T09%3A06%3A00.000Z&_facets=signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

But it would also make sense to strip the prefixes on Windows (RtlAcquireSRWLockExclusive) and OS X (pthread_mutex_lock, pthread_mutex_trylock or __pthread_mutex_lock).

I'm not sure what you mean by "strip". That's not a word we use in signature generation algorithm and there are a few different things that result in a signature that doesn't have that symbol.

Documentation for signature generation is here:

https://socorro.readthedocs.io/en/latest/signaturegeneration.html

with the particular bits I think are pertinent here being here:

https://socorro.readthedocs.io/en/latest/signaturegeneration.html#signature-generation-algorithm

We could make that symbol a sentinel. If we did that, then the signature would start with symbols after the mozilla::detail::MutexImpl::mutexLock symbol. We do that with some of the panic symbols since we want to skip all the panic machinery.

Here's an example:

• before: libc.so@0xb0318 | mozilla::detail::MutexImpl::mutexLock | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot
• after: mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot

We could add that symbol to the irrelevant list. If we did that, then the symbol wouldn't show up in the signature at all.

• before: libc.so@0xb0318 | mozilla::detail::MutexImpl::mutexLock | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot
• after: libc.so@0xb0318 | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot

You mention the libc.so@0xb0318 symbol being problematic, so I suspect you want to make mozilla::detail::MutexImpl::mutexLock a sentinel.

Does that sound like what you're looking for?

Sorry for my bad terminology! Originally, I was thinking of:

• before: libc.so@0xb0318 | mozilla::detail::MutexImpl::mutexLock | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot
• after: mozilla::detail::MutexImpl::mutexLock | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot
  or (theoretically, I don't think we have exactly this case)
• before: libc.so@0xb0318 | trunc | mozilla::detail::MutexImpl::mutexLock | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot
• after: mozilla::detail::MutexImpl::mutexLock | mozilla::layers::CompositorBridgeParent::GetGeckoContentControllerForRoot

and therefore remove/strip everything before mozilla::detail::MutexImpl::mutexLock.

However, removing anything up to and including mozilla::detail::MutexImpl::mutexLock seems to be fine as well. In particular, since this might well get inlined in some build variants.

So, yes, making mozilla::detail::MutexImpl::mutexLock a sentinel sounds good!

Cool! No worries about terms--I just want to make sure we're talking about the same thing.

Grabbing this to work on today.

Simon: mozilla::detail::MutexImpl::mutexLock is in the prefix list, so it gets included in the signature. If I add that symbol to the sentinels list, then we get this:

app@socorro:/app$ socorro-cmd signature 846be766-c658-44e3-b843-feedf0210121
Crash id: 846be766-c658-44e3-b843-feedf0210121
Original: libc.so@0x90c70 | mozilla::detail::MutexImpl::mutexLock | mozilla::MozPromise<T>::ThenInternal | mozilla::MozPromise<T>::ThenValue<T>::DoResolveOrRejectInternal
New:      mozilla::detail::MutexImpl::mutexLock | mozilla::MozPromise<T>::ThenInternal | mozilla::MozPromise<T>::ThenValue<T>::DoResolveOrRejectInternal
Same?:    False

I can also remove mozilla::detail::MutexImpl::mutexLock from the prefix list and then it won't show up in the signature, if you want.

Which do you prefer?

Then let it stay in the prefix list. That actually matches my original idea then.

Depending on the situation, either way might be better, but by letting it on the prefix list, the signatures can still be distinguished where relevant, and it's now comparatively easy to check two signatures for cases where the two signatures actually cover the same case.

willkg merged PR #5657: "bug 1687907: add mozilla::detail::MutexImpl::mutexLock to sentinels" in 05d9e38.

This was pushed to production in bug #1688681.

Simon: Want me to reprocess these crash reports?

Given that we could get a better overview of the crashes of Bug 1687597 then, reprocessing would be great!

I reprocessed them going back 6 months. Marking this as FIXED.

Hm, that does not seem to have worked. I still see signatures such as https://crash-stats.mozilla.org/signature/?signature=RtlAcquireSRWLockExclusive%20%7C%20mozilla%3A%3Adetail%3A%3AMutexImpl%3A%3Alock%20%7C%20mozilla%3A%3Adom%3A%3AWorkerPrivate%3A%3AResetWorkerPrivateInWorkerThread&date=%3E%3D2020-10-28T12%3A14%3A00.000Z&date=%3C2021-01-28T12%3A14%3A00.000Z&_sort=-date

Or am I misunderstanding something here?

This bug was for adding mozilla::detail::MutexImpl::mutexLock and the signature you're asking about has mozilla::detail::MutexImpl::lock.

I can add mozilla::detail::MutexImpl::lock as well. Does that help?

Oh, my fault, I didn't see this "subtle" difference in the signatures.

I now checked all signatures containing mozilla::detail::MutexImpl::. In the posix implementation, mozilla::detail::MutexImpl::lock just calls mozilla::detail::MutexImpl::mutexLock, and either of those therefore gets always inlined (or almost always?). In the Windows implementation however, there is no mozilla::detail::MutexImpl::mutexLock.

The same signature stuff also applies to mozilla::detail::MutexImpl::unlock (there's no mozilla::detail::MutexImpl::mutexUnlock).

It could also apply to mozilla::detail::MutexImpl::tryLock and mozilla::detail::MutexImpl::mutexTryLock, but there are very few crashes for that.

If it's not much of an additional effort, it would be good to do the same for:

• mozilla::detail::MutexImpl::lock
• mozilla::detail::MutexImpl::unlock
• mozilla::detail::MutexImpl::tryLock
• mozilla::detail::MutexImpl::mutexTryLock

If it's extra effort, the last two might be left out for now.

Sorry again for not looking more closely at this from the beginning.

Signature generation is an exercise in tinkering, so it's totally fine to go back and forth over iterations. Signature generation is also pretty easy to tinker with. It's not much effort to try some more variations. So, it's all good! :)

I'll reopen this and tinker with comment #13 ideas either today or Monday.

Example crash reports:

• mozilla::detail::MutexImpl::lock: bp-c3d49021-9800-4179-8aa2-da3b80210202
• mozilla::detail::MutexImpl::unlock: bp-ed1edbbb-fe64-46e7-94a6-dfe2c0210202
• mozilla::detail::MutexImpl::tryLock: bp-5fc5d064-fae8-4147-a54c-b79360210128
• mozilla::detail::MutexImpl::mutexTryLock: bp-8192b329-b002-40df-8c90-a55010210118

willkg merged PR #5672: "bug 1687907: add more mozilla::detail::MutexImpl:: sentinels" in b35bc1e.

I deployed this earlier today as part of bug #1690378. I reprocessed 222,000 or so crash reports that had mozilla::detail::MutexImpl in the signature.

Marking this as FIXED.