Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented sys_game_watchdog syscalls #12815

Merged
merged 1 commit into from Oct 17, 2022
Merged

Implemented sys_game_watchdog syscalls #12815

merged 1 commit into from Oct 17, 2022

Conversation

brian218
Copy link
Contributor

I implemented three sys_game_watchdog syscalls (sys_game_watchdog_start, sys_game_watchdog_stop, and sys_game_watchdog_clear) as a few games seem to rely on the watchdog to reboot the game itself.

@brian218
Copy link
Contributor Author

Needs more suggestions

@brian218 brian218 marked this pull request as ready for review October 15, 2022 20:04
@Megamouse Megamouse removed their request for review October 15, 2022 20:08
rpcs3/Emu/Cell/lv2/sys_game.cpp Outdated Show resolved Hide resolved
rpcs3/Emu/Cell/lv2/sys_game.cpp Outdated Show resolved Hide resolved
@brian218 brian218 requested review from Nekotekina and Megamouse and removed request for Megamouse and Nekotekina October 16, 2022 08:33
@brian218 brian218 requested review from Nekotekina and removed request for Megamouse October 17, 2022 17:09
@Nekotekina Nekotekina merged commit 5c24009 into RPCS3:master Oct 17, 2022
@elad335
Copy link
Contributor

elad335 commented Oct 17, 2022

Uh, there are thread safety issues and bugs with emulation stopping at this state.

@brian218
Copy link
Contributor Author

Uh, there are thread safety issues and bugs with emulation stopping at this state. I'm also not sure why Emu.Restart() was choosed over Emu.Stop().

The watchdog is supposed to restart the game. Also, I've observed that some arcade games rely on the watchdog to make the game itself restarted, by stopping calling sys_game_watchdog_clear().
As for the thread safety, I adopted Emu.CallFromMainThread([](){Emu.Restart();}); so the Emu.Restart() will be called from the GUI thread, as if the player clicked "Restart" in the RPCS3 menu -> Emulation.

@elad335
Copy link
Contributor

elad335 commented Oct 17, 2022

As for the emulation stopping bug, I was referring to the fact that the watchdog thread operation leaks even after GUI emulation stopping request is being issued, potentially restarting other applications in this time frame.

@brian218
Copy link
Contributor Author

brian218 commented Oct 17, 2022

As for the emulation stopping bug, I was referring to the fact that the watchdog thread operation leaks even after GUI emulation stopping request ie veing issued, potentially restarting other applications in this time frame.

How about changing std::this_thread::sleep_for(1s); to a smaller value? or adding _sys_game_watchdog_stop() to the game shutdown process of RPCS3?
Also, the watchdog won't issue the restart request when Emu.IsStopped() == true.

@elad335
Copy link
Contributor

elad335 commented Oct 17, 2022

Neither, it should be manged by a named_thread held by FXO and joined on emulation stopping implicitly by its destructor.

@elad335
Copy link
Contributor

elad335 commented Oct 17, 2022

Also, the watchdog won't issue the restart request when Emu.IsStopped() == true.

This is also a race condition.

@brian218
Copy link
Contributor Author

Neither, it should be manged by a named_thread held by FXO and joined on emulation stopping implicitly by its destructor.

Hmm. Maybe I can make another PR to optimize it later. Would you mind providing some examples of your suggestion?

@brian218
Copy link
Contributor Author

If I understand correctly, you want the watchdog thread to be terminated/destroyed immediately right after the game is stopped, right?

@elad335
Copy link
Contributor

elad335 commented Oct 17, 2022

Yes, and make the state of watchdog packed in a single atomic variable and updated atomically with fused error checking with its single atomic update.

@brian218
Copy link
Contributor Author

Neither, it should be manged by a named_thread held by FXO and joined on emulation stopping implicitly by its destructor.

Hmm. Maybe I can make another PR to optimize it later. Would you mind providing some examples of your suggestion?

Any examples would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants