Collect number of process FDs and /proc/self/limits#90
Collect number of process FDs and /proc/self/limits#90Jake-Shadle merged 6 commits intorust-minidump:mainfrom
Conversation
ad40d66 to
4ccc432
Compare
Jake-Shadle
left a comment
There was a problem hiding this comment.
Is there some prior work you can point to for this? AFAICT neither breakpad nor crashpad do something similar, so it would help to know why this change is useful. In particular the /proc/{crash_thread}/fds doesn't really seem very useful as it only records the number of files, not their names/attributes/etc.
So we have had an unexplained rise of Either way, we lack proper view to assert the root cause here, and collecting |
|
(I should probably have opened it as a draft PR though) |
|
Ok, that makes sense. I think this can be merged as is, but I think before it is released (once rust-minidump/rust-minidump#881 is merged and released) it would make sense to spec out the contents of the streams, for example changing the FDs stream to be the true companion of the limits stream, with one field for each limit as well as a version header so that the fields can be changed over time, as things like OOM killing could also be inferred from this information, as limiting to just FDs feels too specific for your particular use case. |
|
FYI I'm looking at the existing structures that are used for Windows, maybe we could repurpose one to store the fd count like we do for other pieces of information. |
|
Yah, that could work as well, just feels too specific and limiting to have an entire stream for a single number. |
Thanks for the explanation! Since we already capture |
that's indeed one of the things I considered, but when I tested, ~28 opened FDs would show up as |
|
We've discussed this a bit offline and I came up with an idea to gather richer information that would still help us explain the Firefox issue we're dealing with. Windows minidumps have introduced in recent times the |
|
BTW I'm unsure what do to about aff27cf ? |
|
@Jake-Shadle should I merge PR #94 first? |
|
@lissyx please keep it, as the comment says, if you link with LLD it will by default use a 64 bit hash for the build id which causes some tests to fail, but it can't be on by default since the flags only work on LLD and not ld/gold. |
|
@gabrielesvelto sure, then this PR can rebase and bump the stream count and get merged. |
this actually depends on rust-minidump/rust-minidump#881 to the
minidump-commoncrate would need to be updated