Skip to content

Commit

Permalink
fix(sdk): synchronize packet sending
Browse files Browse the repository at this point in the history
Prevents obscure XStream conversion issues that occurred when messages
interleaved.
  • Loading branch information
xeruf committed Feb 10, 2021
1 parent 40789a2 commit 219466c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
15 changes: 10 additions & 5 deletions sdk/src/server-api/sc/networking/clients/XStreamClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,17 @@ public void sendCustomData(byte[] data) throws IOException {
networkInterface.getOutputStream().flush();
}

public void send(ProtocolMessage packet) {
public synchronized void send(ProtocolMessage packet) {
if (!isReady())
throw new IllegalStateException("Please call start() before sending any packets!");

if (isClosed())
throw new IllegalStateException("Writing on a closed xStream!");
throw new IllegalStateException(
String.format("Trying to write packet on %s which wasn't started: %s", shortString(), packet));

if (isClosed()) {
logger.warn("Writing on a closed Stream -> dropped the packet (tried to send package of type {}) Thread: {}",
packet.getClass().getSimpleName(),
Thread.currentThread().getName());
return;
}

logger.debug("{}: Sending {} via {} from {}", shortString(), packet, networkInterface, toString());
if (logger.isTraceEnabled())
Expand Down
39 changes: 15 additions & 24 deletions server/src/sc/server/network/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,6 @@ public void addRole(IClientRole role) {
this.roles.add(role);
}

/** Send a package to the server. */
@Override
public synchronized void send(ProtocolMessage packet) {
if (!isClosed()) {
super.send(packet);
} else {
logger.warn("Writing on a closed Stream -> dropped the packet (tried to send package of type {}) Thread: {}",
packet.getClass().getSimpleName(),
Thread.currentThread().getName());
}
}

/** Call listener that handle new Packages. */
private void notifyOnPacket(Object packet) throws UnprocessedPacketException {
/*
Expand Down Expand Up @@ -103,18 +91,18 @@ private void notifyOnPacket(Object packet) throws UnprocessedPacketException {
}

/** Call listeners upon error. */
private synchronized void notifyOnError(ProtocolErrorMessage packet) {
private void notifyOnError(ProtocolErrorMessage packet) {
for (IClientListener listener : new ArrayList<>(clientListeners)) {
try {
listener.onError(this, packet);
} catch (Exception e) {
logger.error("OnError Notification caused an exception.", e);
logger.error("OnError Notification caused an exception, error: " + packet, e);
}
}
}

/** Call listeners upon disconnect. */
private synchronized void notifyOnDisconnect() {
private void notifyOnDisconnect() {
if (!this.notifiedOnDisconnect) {
this.notifiedOnDisconnect = true;
for (IClientListener listener : new ArrayList<>(clientListeners)) {
Expand Down Expand Up @@ -142,9 +130,11 @@ public void removeClientListener(IClientListener listener) {
* @return true iff this client has an AdministratorRole
*/
public boolean isAdministrator() {
for (IClientRole role : this.roles) {
if (role instanceof AdministratorRole) {
return true;
synchronized(roles) {
for (IClientRole role : this.roles) {
if (role instanceof AdministratorRole) {
return true;
}
}
}
return false;
Expand Down Expand Up @@ -179,12 +169,13 @@ public void authenticate(String password) throws AuthenticationFailedException {
*/
@Override
protected void onDisconnect(DisconnectCause cause) {
super.onDisconnect(cause);
for (IClientRole role : this.roles) {
try {
role.disconnect(cause);
} catch (Exception e) {
logger.warn("Couldn't close role.", e);
synchronized(roles) {
for (IClientRole role : this.roles) {
try {
role.disconnect(cause);
} catch (Exception e) {
logger.warn("Couldn't close role.", e);
}
}
}

Expand Down

0 comments on commit 219466c

Please sign in to comment.