Always CAS the disconnecting variable when we disconnect.

When we changed the disconnect check to use proper concurrent behavior (using CAS), the disconnecting flag wasn't set the first time we disconnect because the OR short-circited the CAS.

Upstream doesn't have this issue because they don't set the flag in the if statement (with CAS), and therefore aren't short circuted.

I have no idea why this check was an OR statement in the first place, so maybe this will cause more crazy bugs.
This commit is contained in:
Techcable 2016-05-31 12:01:46 -06:00
parent 05fc872738
commit 2187ad029c
No known key found for this signature in database
GPG Key ID: 091A03B91D7FCE68
5 changed files with 35 additions and 55 deletions

View File

@ -1,4 +1,4 @@
From 6b2815428c47ffb0e21f8a9f2409da37ac648191 Mon Sep 17 00:00:00 2001 From d473177e5c6407f0f0c362bad49da0fbc7a45f19 Mon Sep 17 00:00:00 2001
From: kamcio96 <k.nadworski@icloud.com> From: kamcio96 <k.nadworski@icloud.com>
Date: Mon, 14 Mar 2016 15:59:52 -0700 Date: Mon, 14 Mar 2016 15:59:52 -0700
Subject: [PATCH] Improve connection closing, fixing the kick delay. Subject: [PATCH] Improve connection closing, fixing the kick delay.
@ -49,9 +49,32 @@ index c88ab49..9a13f5c 100644
if ( server != null ) if ( server != null )
{ {
diff --git a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java diff --git a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java
index d841090..b27b447 100644 index d841090..8c19974 100644
--- a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java --- a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java
+++ b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java +++ b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java
@@ -8,6 +8,7 @@ import java.net.URLEncoder;
import java.security.MessageDigest;
import java.util.List;
import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import javax.crypto.SecretKey;
import com.google.gson.Gson;
@@ -96,12 +97,11 @@ public class InitialHandler extends PacketHandler implements PendingConnection
private boolean legacy;
@Getter
private String extraDataInHandshake = "";
- private boolean disconnecting;
+ private final AtomicBoolean disconnecting = new AtomicBoolean(false);
- @Override
public boolean shouldHandle(PacketWrapper packet) throws Exception
{
- return !disconnecting;
+ return !disconnecting.get();
}
private enum State
@@ -136,8 +136,7 @@ public class InitialHandler extends PacketHandler implements PendingConnection @@ -136,8 +136,7 @@ public class InitialHandler extends PacketHandler implements PendingConnection
public void handle(LegacyHandshake legacyHandshake) throws Exception public void handle(LegacyHandshake legacyHandshake) throws Exception
{ {
@ -82,10 +105,14 @@ index d841090..b27b447 100644
} }
@Override @Override
@@ -538,24 +535,11 @@ public class InitialHandler extends PacketHandler implements PendingConnection @@ -535,27 +532,13 @@ public class InitialHandler extends PacketHandler implements PendingConnection
if ( !disconnecting || !ch.isClosed() ) @Override
public void disconnect(final BaseComponent... reason)
{ {
disconnecting = true; - if ( !disconnecting || !ch.isClosed() )
+ if ( !ch.isClosed() && disconnecting.compareAndSet(false, true) )
{
- disconnecting = true;
- // Why do we have to delay this you might ask? Well the simple reason is MOJANG. - // Why do we have to delay this you might ask? Well the simple reason is MOJANG.
- // Despite many a bug report posted, ever since the 1.7 protocol rewrite, the client STILL has a race condition upon switching protocols. - // Despite many a bug report posted, ever since the 1.7 protocol rewrite, the client STILL has a race condition upon switching protocols.
- // As such, despite the protocol switch packets already having been sent, there is the possibility of a client side exception - // As such, despite the protocol switch packets already having been sent, there is the possibility of a client side exception

View File

@ -1,47 +0,0 @@
From cb5d2edbea4ce576f91f2e7b23bac8330b268d33 Mon Sep 17 00:00:00 2001
From: Tux <write@imaginarycode.com>
Date: Sun, 20 Mar 2016 01:36:23 -0400
Subject: [PATCH] Better race condition check for disconnect()
diff --git a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java
index b27b447..13a549a 100644
--- a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java
+++ b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java
@@ -8,6 +8,7 @@ import java.net.URLEncoder;
import java.security.MessageDigest;
import java.util.List;
import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import javax.crypto.SecretKey;
import com.google.gson.Gson;
@@ -96,12 +97,11 @@ public class InitialHandler extends PacketHandler implements PendingConnection
private boolean legacy;
@Getter
private String extraDataInHandshake = "";
- private boolean disconnecting;
+ private final AtomicBoolean disconnecting = new AtomicBoolean(false);
- @Override
public boolean shouldHandle(PacketWrapper packet) throws Exception
{
- return !disconnecting;
+ return !disconnecting.get();
}
private enum State
@@ -532,9 +532,8 @@ public class InitialHandler extends PacketHandler implements PendingConnection
@Override
public void disconnect(final BaseComponent... reason)
{
- if ( !disconnecting || !ch.isClosed() )
+ if ( disconnecting.compareAndSet(false, true) || !ch.isClosed() )
{
- disconnecting = true;
if ( thisState != State.STATUS && thisState != State.PING ) {
ch.close( new Kick( ComponentSerializer.toString( reason ) ) );
} else {
--
2.8.3

View File

@ -1,4 +1,4 @@
From 0ee1851dd10b3146541b045ae233b03f74381291 Mon Sep 17 00:00:00 2001 From b9e07cfb20542a5db8f08539c6472c3178e6e39a Mon Sep 17 00:00:00 2001
From: Techcable <Techcable@techcable.net> From: Techcable <Techcable@techcable.net>
Date: Tue, 5 Apr 2016 11:00:16 -0700 Date: Tue, 5 Apr 2016 11:00:16 -0700
Subject: [PATCH] Print stack trace when the ByteBuf is not direct. Subject: [PATCH] Print stack trace when the ByteBuf is not direct.

View File

@ -1,4 +1,4 @@
From 0189e040175b33bfaaf9315fd902144d78470041 Mon Sep 17 00:00:00 2001 From 082cb79ea1bf91ec488e39b30e7178d7db515f71 Mon Sep 17 00:00:00 2001
From: Tux <write@imaginarycode.com> From: Tux <write@imaginarycode.com>
Date: Wed, 13 Apr 2016 14:00:40 -0400 Date: Wed, 13 Apr 2016 14:00:40 -0400
Subject: [PATCH] Validate that chat messages are non-blank Subject: [PATCH] Validate that chat messages are non-blank

View File

@ -1,4 +1,4 @@
From 7c7f35b10a044315fc067c505f1785585e721201 Mon Sep 17 00:00:00 2001 From 19c64c2c2af38732cebc271aef220a5f894135aa Mon Sep 17 00:00:00 2001
From: Techcable <Techcable@techcable.net> From: Techcable <Techcable@techcable.net>
Date: Mon, 25 Apr 2016 23:46:00 -0700 Date: Mon, 25 Apr 2016 23:46:00 -0700
Subject: [PATCH] Reduce the overhead of lots and lots of teams with the same Subject: [PATCH] Reduce the overhead of lots and lots of teams with the same