Add option to terminate a system via its process name

[byancey]

This commit adds an option to the System Dialog to allow a sytem
to be killed via process name. This augments the existing options
to terminate by closing the window or terminate killing the process
by it's handle.

This was added to address an issue in Pinball FX3 where the game
will hang upon exit instead of closing gracefully if it was
originally launched with one of the "-hotseat_x" options. The
issue was reported as issue #11 on github back in Oct. 2018 but
no resolution was identified at that time.

It's unclear why the built-in TerminateProcess() call is unable
to force Pinball FX3 to terminate, however, if I explicitly locate
a handle based on the process name, and then terminate that handle
via TerminateProcess(), the process immediatly terminates and returns
control to PinballY.

[mjrgh]

It's unclear why the built-in TerminateProcess() call is unable to force Pinball FX3 to terminate...

Glad you found a solution. At a guess, maybe FX3 is launching multiple processes in stages, so the one that we initially identify isn't the one that's still running at the end.

[sebinouse]

Good job ! Thanks a lot

[mjrgh]

Before I merge this, could you try something for me? I want to see if it's really needed or if it was just an oversight in the original formulation. The thing is that you're doing almost exactly the same thing the program was doing originally, so it seems redundant to do it twice like this.

The thing to try is: in the original process scan in Application::GameMonitorThread::Main(), around line 3146, try adding the PROCESS_TERMINATE bit to the access request in the OpenProcess call:

	HandleHolder newProc = OpenProcess(
		PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE | PROCESS_TERMINATE, 
		false, procInfo.th32ProcessID);

Then go back and test FX3 hotseat mode with the original Kill Process option setting (instead of your new one).

It might really be necessary to do a second lookup, but I wanted to make sure it wasn't just this before adding the extra complexity.

Additional question: any reason you're using return code 9 in the TerminateProcess() call in the new section?

[byancey]

The exit code 9 was a throwback to SIGKILL, but I can't recall if I started with 9, or if I tried 0 first and needed 9 to actually get the process to die.

I'll run the test with your suggestion above. If based on that, my kill process fix is still needed, I'll test again with return code 0 and see if that also works.

I may be able to get to this tonight, but it's possible it could be a few days.

[mjrgh]

The exit code 9 was a throwback to SIGKILL

That's what I was guessing. :) With Win32 TerminateProcess, of course, this isn't that sort of signal code, it's just the exit code that you want the process to pass back to parents/others doing a Wait on its process handle, as though the process itself had executed "exit(9)" from one of its threads. I don't think there's any way it can make a difference in whether or not TerminateProcess() will succeed, but I wanted to ask just in case you had uncovered some secret Windows internal I've never heard of. (There are certainly many of those!)

I'll run the test with your suggestion above. If based on that,
my kill process fix is still needed, I'll test again with return code
0 and see if that also works.

Great, look forward to hearing how it goes.

[byancey]

Sorry it took so long to get around to testing your suggestion. Unfortunately, it did not make a difference in the behavior when attempting to exit Pinball FX3 from hotseat mode.

I also verified that the return code of TerminateProcess was not relevant to the behavior of my fix and I have submitted a 2nd commit which changes the return code to 0.

I see there is currently a merge conflict due to the changes to the OptionsDialog.rc, which is treated as a binary by git. The only change I made there was to add and additional entry "Kill Process By Name (KillProcessByName)" to the "Terminate By" drop down in the dialog. This option is then picked up by GetLaunchParam in Application::GameMonitorThread::CloseGame(). Please let me know if there's anything else you need from me on this.

[mjrgh]

Great - thanks for giving that a try. I'll go ahead and merge this.

One additional experiment I'd be keen to try, if you have a chance at some point, is to capture the process ID of the original process handle that the existing code is grabbing, and compare it to the process ID of the new handle that you're finding on the second scan. That would test the theory that FX3 in hot seat mode really is launching a THIRD process (Steam -> Initial FX process -> Second FX process) and explain without a doubt why the existing code can't kill the process. If it turns out that it's the same process ID for both handles, it would suggest that there's some other problem in the existing code that we might be better off fixing. (Better off in the sense that it wouldn't foist the problem onto the user by forcing them to pick the right magic option.)

[mjrgh]

Hi - just wanted to check if you've had a chance to try the process ID comparison. I want to make sure we've confirmed that's what's going on, so we're sure we understand the real issue.