Server crash UUID lookup #28
Assignees
minster586
Clear assignees
minster586
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: minster586/OpenInv#28
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A player used /inv offlineplayer, which immediately hogged the servers main thread, resulting in a complete crash.
Thread dump stacktrace: http://pastebin.com/8bzySypz
@ShadowRanger seems like the UUID util here. We should add a boolean that prevents the UUID util from looking it up from MC servers and instead rely on just local player lists.
Alright ill look into this a bit later today.
I can't seem to reproduce this problem. I know it's to do with the UUID fetcher system but is it for offline servers or something? Not sure what's causing the error because it currently falls back to using getOfflinePlayer for getting UUID's if it fails with the fetcher.
We shouldn't be doing network calls from the Main thread. If the internet gets slow, or stuck, then we would be stalling the whole server...
UUIDFetcher runs on a separate thread already, not sure why it would be running on the main?
I don't think it's running on another thread. At least not according to the trace or the code.
The UUIDFetcher is run through a Callable, so whenever it's call() method is executed, it should perform the logic on another thread.
A callable is only an interface, so there's nothing that actually spawns a separate thread. The stacktrace also passes through directly:
com.lishid.openinv.utils.UUIDFetcher.writeBody(UUIDFetcher.java:57)
com.lishid.openinv.utils.UUIDFetcher.call(UUIDFetcher.java:40)
com.lishid.openinv.utils.UUIDUtil.getUUIDOf(UUIDUtil.java:51)
Even if we implement a threading model, we still need to wait on the thread to finish the call to get the result, which is horrible either way since we stall the main thread. I recommend a flag that would restrict getting UUID to only local players, and set it to true when we are on the main thread (handling any events and commands)
Yeah right sorry I think I'm confusing myself. Didn't realize it was being run on the main thread.
What if we run the fetcher async with the Bukkit scheduler instead?
The problem is that we need it to return the result to the caller of the method, and there's no way that the value is returned instantly since we need to wait for the network call to succeed, but we can't do any waiting on the main thread at all without potentially impacting TPS. I think because of this we'll have to skip the network fetch if we're on the main thread.
Alright yeah, I understand what you're saying. I'll see what I can produce and let you know how it goes.
As for determining whether or not we're on the main thread, do you think using Bukkit.getServer().isPrimaryThread() could be a good option instead of a custom system using flags when running events/commands?
That sounds ok as well, but I'd say that having an explicit option means
that we can control it if we're on another thread but don't want to be
blocked. Either way it should be fine.
On Jul 10, 2015 1:58 AM, "ShadowRanger" notifications@github.com wrote:
Okay so doing it with flags would you mean something like having a static boolean in the OpenInv main class and then set it to true before any of our logic is executed in the listeners and commands and then back to false once finished?
Can't we just add a parameter to the call...
On Jul 10, 2015 2:29 AM, "ShadowRanger" notifications@github.com wrote:
Sorry if I sound ignorant but a parameter to which call?
Oh I mean when you call UUIDUtil, pass an extra boolean flag.
On Jul 10, 2015 2:33 AM, "ShadowRanger" notifications@github.com wrote:
Thanks for fixing it, I guess this can be closed?