From f17e0efd078bc284705851e12181d9409123db1e Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Tue, 4 Nov 2008 19:55:06 +0000 Subject: [PATCH] The throttle update cannot be applied in a Runnable, or we get a race condition against the frames coming in from network. The actual _throttle is only ever access from the network thread, so only _pendingThrottles needs extra care. git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@5501 542714f4-19e9-0310-aa3c-eee0fc999fb1 --- .../presents/server/PresentsClient.java | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/java/com/threerings/presents/server/PresentsClient.java b/src/java/com/threerings/presents/server/PresentsClient.java index e6d69487f..1b062ddc0 100644 --- a/src/java/com/threerings/presents/server/PresentsClient.java +++ b/src/java/com/threerings/presents/server/PresentsClient.java @@ -190,7 +190,9 @@ public class PresentsClient @EventThread public void setIncomingMessageThrottle (int messagesPerSec) { - _pendingThrottles.add(messagesPerSec); + synchronized(_pendingThrottles) { + _pendingThrottles.add(messagesPerSec); + } postMessage(new UpdateThrottleMessage(messagesPerSec)); // when we get a ThrottleUpdatedMessage from the client, we'll apply the new throttle } @@ -849,23 +851,23 @@ public class PresentsClient */ protected void throttleUpdated () { - _omgr.postRunnable(new Runnable() { - public void run () { - if (_pendingThrottles.size() == 0) { - log.warning("Received throttleUpdated but have no pending throttles", - "client", this); - return; - } - int messagesPerSec = _pendingThrottles.remove(0); - log.info("Applying updated throttle", "client", this, "msgsPerSec", messagesPerSec); - // we set our hard throttle over a 10 second period instead of a 1 second period to - // account for periods of network congestion that might cause otherwise properly - // throttled messages to bunch up while they're "on the wire"; we also add a one - // message buffer so that if the client is right up against the limit, we don't end - // up quibbling over a couple of milliseconds - _throttle.reinit(10*messagesPerSec+1, 10*1000L); + int messagesPerSec; + synchronized(_pendingThrottles) { + if (_pendingThrottles.size() == 0) { + log.warning("Received throttleUpdated but have no pending throttles", + "client", this); + return; } - }); + messagesPerSec = _pendingThrottles.remove(0); + } + + log.info("Applying updated throttle", "client", this, "msgsPerSec", messagesPerSec); + // We set our hard throttle over a 10 second period instead of a 1 second period to + // account for periods of network congestion that might cause otherwise properly + // throttled messages to bunch up while they're "on the wire"; we also add a one + // message buffer so that if the client is right up against the limit, we don't end + // up quibbling over a couple of milliseconds + _throttle.reinit(10*messagesPerSec+1, 10*1000L); } @Override