Add OS.get_process_exit_code() method

[KoBeWi]

Closes godotengine/godot-proposals#9471
The method is implemented for Windows, Linux, Mac OS, Android. I only tested Windows.

[Calinou]

it could be implemented for Android, iOS and macOS too.

OS_Unix is the base class of OS_LinuxBSD and OS_MacOS, so the implementation added by this PR should already be used on both macOS and Linux.

[KoBeWi]

Yeah, I forgot to update the description.

[lyuma]

The semantics of waitpid on POSIX style operating systems might need to be explained:
Specifically, not calling get_process_exit_code() may imply that child processes remain in a zombie state, since they require the some sort of wait call to reap the PID... and get_process_exit_code is evidently the call which reaps the zombie.

And calling get_process_exit_code() more than once will fail because a PID can't be reaped more than once.

not saying we need to change the design necessarily, but it might be good to explain these points in the docs. (Or if someone else with good understanding of POSIX style APIs can offer a different explanation)

[bruvzg]

The semantics of waitpid on POSIX style operating systems might need to be explained:
Specifically, not calling get_process_exit_code() may imply that child processes remain in a zombie state, since they require the some sort of wait call to reap the PID... and get_process_exit_code is evidently the call which reaps the zombie.

And calling get_process_exit_code() more than once will fail because a PID can't be reaped more than once.

It is the same for Windows (but instead of kernel, we keep process handles in the process_map), everything is cleared after parent process terminates in both cases. The only part that might be worth mentioning is about calling this and is_process_running methods more than one (after process termination).

[akien-mga]

Would it make sense to cache the result of querying the exit code or whether the process is running so that calling it multiple times does not cause issues? Or if the OS signals the termination of the process somehow, automatically query and cache it so that those two methods just do system calls while the process is running, and provide the cached result otherwise?

I don't know if that makes sense but it could be a more user-friendly API than purely exposing the system calls with their quirks.

[bruvzg]

Would it make sense to cache the result of querying the exit code or whether the process is running so that calling it multiple times does not cause issues? Or if the OS signals the termination of the process somehow, automatically query and cache it so that those two methods just do system calls while the process is running, and provide the cached result otherwise?

It might be worth caching status (in both is_process_running and get_process_exit_code), since something like this might be used (and won't work w/o caching):

if !OS.is_process_running(...):
    int exit_code = OS.get_process_exit_code(...)
    ...

[KoBeWi]

I pushed caching for Windows, but I'm getting same results with and without it.
OSUnix doesn't have convenient process map that can be used for cache.

[akien-mga]

OSUnix doesn't have convenient process map that can be used for cache.

@bruvzg Should we add a similar process map to OS_Unix to achieve that?

[bruvzg]

I pushed caching for Windows, but I'm getting same results with and without it.

It should not be necessary on Windows, only for Unix (I guess as HashMap<pid, status> similarly to Windows process_map).

[KoBeWi]

I pushed process map for Unix (not tested).

I guess as HashMap<pid, status>

I made it a struct, because it needs information whether the process is still running (unless there is some exit code that can be assumed to never be returned from app?).

[lyuma]

I am concerned about the additional complexity needed to support this order of operations. In particular, it needs to work from any thread.

Also if there is nothing to clean up data from this HashMap, will it leak?

My personal preference would be to keep the old design and document that you have to pick one or the other function. Use is_process_running only if you do not care about the exit code.

[bruvzg]

I guess the same mutex should be added to Windows code as well. The rest seems fine.

[KoBeWi]

Pushed Windows mutex.

[bruvzg]

Missing mutex lock/unlock around process_map->insert(pid, pi); (2x) and MutextLock in the kill in the Windows code.

[KoBeWi]

I forgot to save the file before pushing 🤦‍♂️

[akien-mga]

Thanks!