Add support for the MINIDUMP_HANDLE_DATA_STREAM to Linux
#94
Conversation
gabrielesvelto
force-pushed
the
linux-handle-data-stream
branch
from
37a2120 to
f4c18f5
Compare
|
This relies on my changes in PR rust-minidump/rust-minidump#885 so I need to land that one first and cut a new release of the |
gabrielesvelto
force-pushed
the
linux-handle-data-stream
branch
2 times, most recently
from
e63eba4 to
f32b32c
Compare
|
At lunch now, will take a look when I get back. |
src/linux/minidump_writer.rs
Outdated
| let dirent = handle_data_stream::write(self, buffer)?; | ||
| dir_section.write_to_file(buffer, Some(dirent))?; | ||
|
|
There was a problem hiding this comment.
I do have a bit of a concern about the potential fallibility of this operation, if it fails we generate no dump at all, even when all previous sections have been dumped successfully, but I don't know if this will matter in real world usage or not. I suppose if Firefox deploys this change and doesn't see it negatively impacting successful dump creation then it's not a big deal, just something I want to bring up.
Actually, I think the linux dumper just needs to be changed so that every section can fallback to being not written if it fails internally due to whatever reason with the hopes that at least some sections can be written so there is at least something being dumped rather than 1 failure leaving the crash with no diagnosable artifact, but that can be discussed separately.
There was a problem hiding this comment.
Actually, I think the linux dumper just needs to be changed so that every section can fallback to being not written if it fails internally due to whatever reason with the hopes that at least some sections can be written so there is at least something being dumped rather than 1 failure leaving the crash with no diagnosable artifact, but that can be discussed separately.
I agree. I've been monitoring all the possible failure modes when writing minidumps in Firefox and not being able to write a section doesn't happen very often... but it does happen. I can make this particular operation fallible in the meantime.
BTW if we discuss making all the non-necessary sections fallible it would be nice to introduce a mechanism to gather multiple errors. Right now we know what will cause a failure but it's an all-or-nothing situation. It would be nice to have a mechanism so that we get a list of errors to distinguish between scenarios like "we could write the minidump but we don't have the file descriptors" or "we could only partially write a minidump, threads X, Y & Z are missing because we couldn't stop them", etc...
There was a problem hiding this comment.
BTW if we discuss making all the non-necessary sections fallible it would be nice to introduce a mechanism to gather multiple errors. Right now we know what will cause a failure but it's an all-or-nothing situation. It would be nice to have a mechanism so that we get a list of errors to distinguish between scenarios like "we could write the minidump but we don't have the file descriptors" or "we could only partially write a minidump, threads X, Y & Z are missing because we couldn't stop them", etc...
Totally agree, I'll open up a separate issue for this....oh wait, I already did and forgot about it #31.
gabrielesvelto
force-pushed
the
linux-handle-data-stream
branch
2 times, most recently
from
1e6ac65 to
883b711
Compare
gabrielesvelto
force-pushed
the
linux-handle-data-stream
branch
from
883b711 to
2d59eb2
Compare
|
@Jake-Shadle I've bumped a couple of dependencies to align this code to rust-minidump (which in turn was aligned with symbolic). If you're OK with those changes I'll merge this. |
gabrielesvelto
force-pushed
the
linux-handle-data-stream
branch
from
106ef88 to
8cbca26
Compare
No description provided.