Implement client watchdog #4081
Conversation
apple502j
force-pushed
the
crash-report-info/client-watchdog
branch
from
d794aa1 to
4de277f
Compare
| } | ||
|
|
||
| public void run(MinecraftClient client) { | ||
| while (client.isRunning()) { |
There was a problem hiding this comment.
Should there not be a sleep in this while loop, there is no need to constantly check the value. See: DedicatedServerWatchdog
| import net.fabricmc.fabric.mixin.client.crash.report.info.MinecraftClientAccessor; | ||
| import net.fabricmc.loader.api.FabricLoader; | ||
|
|
||
| public class ClientWatchdog implements ClientModInitializer { |
There was a problem hiding this comment.
really needs a testmod, id add a client command that sets a bool, and in the client ticket event just have a while (bool)
Well, a watchdog is useful in any part of code that might do infinite loop. |
|
IMO, a watchdog for the integrated server thread (in production) would be vastly more useful than one on the client thread. It's pretty rare that released mods deadlock the render thread. However, it's quite common for the integrated server to freeze with no explanation. |
|
I agree with @embeddedt however I think we also need to be careful to not trigger it too egarly, and maybe not crash to desktop? It could possibly open a UI allowing the user to ingore it? With this PR as is, if you stall the main render thread will windows show the not responding UI? |
|
Ah yeah, integrated server needs a watchdog as well. Will do that. I doubt it will have that much of a side effect if it only applies on dev/is opt-in. If the server is not ticking for 30 seconds, I think it's fair to say it crashed. I don't want to overcomplicate things (remember this is FAPI).
Need to check later. Without this PR it does show that UI. |
|
By UI I mean just a simple miecraft yes/no screen asking the user if they want to quit. I just worry someone is going to want to run a massive command that takes a long time to complete, and 30 seconds wont be enough. |
|
IMO the integrated server watchdog should not be enabled in dev, only in prod. At the very least, it should be disabled if a debugger is attached, otherwise pausing at a breakpoint for a while is enough to trip it, which makes it an inconvenience. |
|
It might have some extra uses in-dev with a shorter delay to ungrab the mouse, especially on Linux. I'd like to see less use of MC's utility methods where the JDK has suitable alternatives to reduce porting efforts. |
Note: will refactor once #4052 is merged.
Client watchdog works similarly to the dedicated server watchdog - but for render thread. When the client ceases to tick for 30 seconds (or
fabric.clientWatchdog.maxTimeMs), the game forcibly crashes. Useful for debugging infinite loops and the like.This feature is currently enabled only in dev (or with
fabric.clientWatchdog.enabled).TODO: