utils/fork: handle termsig in safe_fork

[Bo98]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

When safe_fork fails due to application termination rather than exist, exitstatus will be nil. Instead we should pass more of the object, as it has other methods like termsig for those cases. When you pass ErrorDuringExecution a Process::Status object, it will handle that and provide proper information on the failure, rather than choking because the passed exit status is nil.

It is unfortunately impossible to create a Process::Status object without using Marshal.load. There is no security risk though as this isn't user input.

Fixes #10661.

[BrewTestBot]

Review period will end on 2021-02-24 at 21:42:58 UTC.

[MikeMcQuaid]

I'd rather we didn't use Marshall here, sorry 😭

[MikeMcQuaid]

Suggested change

	error_hash["status"] = Base64.encode64(Marshal.dump(e.status))
	error_hash["status"] = e.status&.exitstatus

and instead never assume that this is non-nil.

[Bo98]

Fair enough on Marshal, but the problem isn't status being nil. It's exitstatus being nil. If exitstatus is nil, then we need to use termsig (see ErrorDuringExecution and its handling of a Process::Status object). So will have to change around the API a bit (both exitstatus and termsig are integers and we need to differentiate between the two if we pass it here).

[MikeMcQuaid]

Changing API around seems fine to me. As long as we assume non-zero status code I don't think preserving this is super critical (but I'll happily see a more comprehensive fix)

[BrewTestBot]

Review period ended.

[Bo98]

This way should be the cleanest/least disruptive.

[MikeMcQuaid]

Looks good so far, nice work on this cleanup!