1884695 - Allow to customize the minidump save path via an environment variable

Status: NEW

(Toolkit :: Crash Reporting, task)

Description

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

As discussed on bug 1882338 (for geckodriver) we need a way to set the path where minidump files are stored when a content or the parent process crashes. This can be done by modifying the minidumpPath of the CrashReport interface:

(In reply to Gabriele Svelto [:gsvelto] from bug 1585036 comment #30)

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #28)

Gabriele, would it be possible to instruct the crash reporter (maybe via such an environment variable) to not use the default minidump folder? I'm happy to file a bug if wanted and possible.

If you can access JS we already have the minidumpPath property which can be used to set the path where minidumps are written. See how we do it for xpcshell tests.

We shouldn't need that for marionette-client given that it internally uses mozcrash. But geckodriver as well as the wptrunner for generic web-platform tests would highly benefit from that.

Comment 1

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

(In reply to Gabriele Svelto [:gsvelto] from bug 1585036 comment #33)

It would be rather easy. When we set the exception handler we use a temporary location for putting the minidumps which is later changed to the final location. We could set that to the contents of an environment variable, but not a preference, it's too early in the startup process for that.

That being said the temporary path is stored in minidumpPath, so the harness could read it and check for crashes under that path before setting it to the desired location.

Moving this discussion from bug 1585036.

Gabriele, could it maybe considered to move the minidump folder permanently when such a potential environment variable is set? That way any client wouldn't have to check different folders but could specify a folder created by its own. Would be helpful for automatic uploads of artifacts without any other required copying of files. If yes, would a name like MOZ_CRASHREPORTER_MINIDUMP_PATH make sense?

We probably could move this bug to the crash reporter component.

Comment 2

[Gabriele Svelto [:gsvelto]]

Yes, if it's an environment variable it could work as a global override to catch all crashes. This would also simplify handling early/late crashes which can potentially end up in different places because of how we set up the paths (if the profile is present or not).

Comment 3

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

Great. So lets move this bug into the crash reporter component than.

How hard would this be to implement?

Comment 4

[Gabriele Svelto [:gsvelto]]

It shouldn't be too hard, but the logic that sets the minidump path is a bit patchy so I have to check.

Comment 5

[Gabriele Svelto [:gsvelto]]

Note to myself (or whoever will implement this). The idea here is that if MOZ_CRASHREPORTER_MINIDUMP_PATH is set then minidumps should always go there, overriding any other setting. That is we shouldn't move the minidumps around so that they end in that folder, we should generate them there directly. This applies to the WER runtime exception module too (which can read environment variables from the crashed process).

Comment 6

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

Gabriele, I assume this has a lower priority for your team? We would just like to know so that we don't wait on it for bug 1882338 and implement it via a workaround for the time being. Thanks.

Comment 7

[Gabriele Svelto [:gsvelto]]

Yes, I should have been clearer about that. We're all in on bug 1620998 and a number of other improvements mostly focused around Android. That'll take some time and I've put most of the other stuff on pause because it's such a big refactoring that it doesn't make sense to make changes now since it will cut out a lot of the existing code.

Comment 8

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

That's totally fine. Thanks for the details. We will then use a workaround for now.