When the server is stopping, the default execution handler method will throw a
RejectedExecutionException in order to prevent further execution, this causes
us to lose the actual kick reason. To mitigate this, we'll use a seperate marked
class in order to gracefully ignore these.
By default, spigot shifts chat over to an unbounded thread pool,
on a normal server, this really offers no gains, the creation of a thread
on submitting to the pool on these servers eats more time vs just running it in
the netty pipeline, however, on servers using plugins which do work in here, there
could be some overall benefits to moving this stuff outside of the pipeline.
In general, this patch does two things:
1) Exposes the core size for the pool, this allows for ensuring that a number of threads
sit around in the pool, mitigating the need for creating new threads; This IS however
caveated, the ThreadPoolExecutor will ONLY create core threads as they're needed, it
just won't allow for us to dip back under the # of core threads, this can potentially
be mitigated by calling prestartCoreThread, however, I'm not sure if there is much justification
for this
2) Exposes a max size for the pool, as stated, by default this is unbounded, for most
servers limiting the size of the pool is going to have 0 effects given how fast chat
is actually processed, this is honestly really just exposed for the misnomers or people
who just wanna ensure that this won't grow over a specific size if chat gets stupidly active
lets debug mode throw a trace in order to potentially see where
such calls are cascading from easier, but, generally, if you see one setBlock
call, you're gonna see more, and this just potentially causes a flood of logs
which can cause issues for slower terminals, etc.
We can limit the flood by just allowing one for a single gen region,
we'll also only gen a trace for the first one, I see no real pressing need
to generate more, given that that would *massively* negate this patch otherwise
This patch buffers the number of logins which will attempt to join
the world per tick, this attempts to reduce the impact that join floods
has on the server
Entities must be dismounted before teleportation in order to avoid
multiple issues in the server with regards to teleportation, shamefully,
too many plugins rely on the events firing, which means that not firing
these events caues more issues than it solves;
In order to counteract this, Entity dismount/exit vehicle events have
been modified to supress cancellation (and has a method to allow plugins
to check if this has been set), noting that cancellation will be silently
surpressed given that plugins are not expecting this event to not be cancellable.
This is a far from ideal scenario, however: given the current state of this
event and other alternatives causing issues elsewhere, I believe that
this is going to be the best soultion all around.
Improvements/suggestions welcome!
Due to the changes in 1.13, clients will send a tab completion request
for all bukkit commands in order to factor in the lack of support for
brigadier and provide backwards support in the API.
Craftbukkit, however; has moved the chat spam limiter to also interact
with the tab completion request, which while good for avoiding abuse,
causes 1.13 clients to easilly be kicked from a server in bukkit due
to this. Removing the spam limit could cause issues for servers, however,
there is no way for servers to manipulate this without blindly cancelling
kick events, which only causes additional complications. This also causes
issues in that the tab spam limit and chat share the same field but different
limits, meaning that a player having typed a long command may be kicked from
the server.
Splitting the field up and making it configurable allows for server owners
to take the burden of this into their own hand without having to rely on
plugins doing unsafe things.
In some enviroments, the channel limit set by spigot can cause issues,
e.g. servers which allow and support the usage of mod packs.
provide an optional flag to disable this check, at your own risk.
GUIs are opened on the client, meaning that the server cannot block them from opening,
However, it is possible to close these GUIs from the server.
Flower pots are also not updated on the client when interaction is cancelled, this patch
also resolves this.
When modifying the world, CB will store a copy of the affected
blocks in order to restore their state in the case that the event
is cancelled. This change only modifies the collection of blocks
in the world by normal means, e.g. not during tree population,
as the potentially marginal overheads would serve no advantage.
CB was using a CraftBlockState for all blocks, which causes issues
should any block that uses information beyond a data ID would suffer
from missing information, e.g. Skulls.
By using CBs CraftBlock#getState(), we will maintain a proper copy of
the blockstate that will be valid for restoration, as opposed to dropping
information on restoration when the event is cancelled.
This patch intends to bump up the time that a client has to reply to the
server back to 30 seconds as per pre 1.12.2, which allowed clients
more than enough time to reply potentially allowing them to be less
tempermental due to lag spikes on the network thread, e.g. that caused
by plugins that are interacting with netty.
We also add a system property to allow people to tweak how long the server
will wait for a reply. There is a compromise here between lower and higher
values, lower values will mean that dead connections can be closed sooner,
whereas higher values will make this less sensitive to issues such as spikes
from networking or during connections flood of chunk packets on slower clients,
at the cost of dead connections being kept open for longer.
In 1.12.2, Mojang moved the processing of ServerboundKeepAlivePacket off the main
thread, while entirely correct for the server, this causes issues with
plugins which are expecting the PlayerQuitEvent on the main thread.
In order to counteract some bad behavior, we will post handling of the
disconnection to the main thread, but leave the actual processing of the packet
off the main thread.
also adding some additional logging in order to help work out what is causing
random disconnections for clients.
Shane Freeder