1677866 - Memory allocated by `cert_storage` rust crate not reported to about:memory

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement, P2)

Description

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

Attachment: Example unreported DMD block

When running the DMD tool to identify heap-unclassified memory in Gecko, I noticed that a significant amount of allocated memory is allocated by the cert_storage rust crate, specifically with stacks resizing hashtables from load_crlite_stash (https://searchfox.org/mozilla-central/rev/277ab3925eae21b419167a34624ec0ab518d0c94/security/manager/ssl/cert_storage/src/lib.rs#457).

On my local machine, this one allocation site amounted to over 8MB of heap-unclassified memory in the parent process due to this field: https://searchfox.org/mozilla-central/rev/277ab3925eae21b419167a34624ec0ab518d0c94/security/manager/ssl/cert_storage/src/lib.rs#457.

These objects should be integrated with our existing memory infrastructure so they do not appear as heap-unclassified in views such as about:memory.

I believe this can be done using a similar strategy to what is taken by webrender and servo using the malloc_size_of crate and some manual integration with the c++ reporting infrastructure.

Comment 1

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

Attachment: ./dmd.py -a -f 24 --allocation-filter cert_storage

Attached a more complete DMD report which is filtered to list all stack frames containing 'cert_storage'.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

To fix this, we should probably use the malloc_size_of / malloc_size_of_derive crates that style and webrender use... Happy to help getting them hooked up, though it should be easy-ish.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:keeler, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Dana Keeler (she/her) [:keeler]]

Martinho, any interest in implementing this?

Comment 5

[R. Martinho Fernandes [:rmf]]

Sure, I can give this a try.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hello R. Martinho. Are you actually working on this bug? Given that there are a lot of those unreported heap unclassified entries in the DMD report it would be good to see them correctly reported.

Comment 8

[R. Martinho Fernandes [:rmf]]

Henrik, I haven't had time to work on this one yet. If you're asking because you want to pick it up yourself, feel free.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

No I won't be able to do it on my own, but I do lots of DMD investigations so that it would be helpful.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Dana, I don't know how complicated / time invasive it is to get implemented but for tracking down heap-unclassified memory it would be a great help to have. It's reported a lot and as such makes it harder to actually find other issues. Maybe someone could have a look at this? Thanks.

Comment 11

[R. Martinho Fernandes [:rmf]]

Hi Henrik. I will start working on this.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks a lot.

Comment 13

[R. Martinho Fernandes [:rmf]]

Attachment: Bug 1677866 - Report memory allocated by `cert_storage` crate r?keeler

Comment 14

[Neha Kochar [:neha]]

Martinho, please add Dana and mccr8 as reviewers when this is ready for review.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hello Martinho, I wanted to ask if you had a bit of time to already check or even work on the review comments from Dana. Thanks!

Comment 16

[R. Martinho Fernandes [:rmf]]

Hi Henry, I was away last week but I'm working on it now.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/faf00b1dcf5d
Report memory allocated by `cert_storage` crate r=keeler,emilio

Comment 18

[Iulian Moraru]

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

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

R. Martinho thanks a lot for getting this landed!