It's another lazy initialization problem. I only check a single
field before initializing two related fields, which can cause
problems when two threads execute handleLogin() concurrently.
If thread A detects that PACKET_LOGIN_CLIENT is null, it updates both
it and LOGIN_GAME_PROFILE. However, thread B may only see the
PACKET_LOGIN_CLIENT update, and still believe LOGIN_GAME_PROFILE is
NULL. Hence why it causes a NullPointerException in issue #39.
Constructing a WrappedWatchableObject with a
net.minecraft.server.ItemStack would cause ProtocolLib to throw an
IllegalArgumentException, even though WrappedWatchableObject.
setValue(Object) accepts ItemStacks without trouble.
This is because WrappedWatchableObject.getUnwrappedType() didn't handle
ItemStacks at all.
At the moment the setPlayers() method will throw a NullPointerException
if the player count has been hidden. This will correctly reset the
player counts before setting the player list and return null in the
getter instead of throwing an exception.
In the current ProtocolLib release (3.1.0) the getFavicon() method will
throw a NullPointerException if the server is not sending a favicon.
This was partly fixed in 3c5482f but there's still no way of checking if
the server is sending a favicon, without checking if the encoded output
of the CompressedImage.toEncodedText() will return a valid result.
This commit will make the favicon getter and setter in the server ping
correctly handle pings with no favicon, by returning null for
getFavicon() (if no favicon will be displayed) and allowing to hide the
favicon by setting the favicon to null.
Calling remove() in the main thread will block the main thread, which
may lead to a deadlock:
http://pastebin.com/L3SBVKzp
ProtocolLib executes this close() method through a PlayerQuitEvent in
the main thread, which has implicitly aquired a lock on
SimplePluginManager (see SimplePluginManager.callEvent(Event)).
Unfortunately, the remove() method will schedule the removal on one of
the Netty worker threads if it's called from a different thread,
blocking until the removal has been confirmed.
This is bad enough (Rule #1: Don't block the main thread), but the real
trouble starts if the same worker thread happens to be handling a server
ping connection when this removal task is scheduled. In that case, it
may attempt to invoke an asynchronous ServerPingEvent
(see PacketStatusListener) using SimplePluginManager.callEvent(). But,
since this has already been locked by the main thread, we end up with a
deadlock. The main thread is waiting for the worker thread to process
the task, and the worker thread is waiting for the main thread to
finish executing PlayerQuitEvent.
TLDR: Concurrenty is hard.
