From b510d373b5476e88014388294d74b911eb9d4983 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 12 Oct 2019 10:22:31 +0200 Subject: [PATCH] reactor: have synchronized block include peeking at scheduled actions If we do not peek at the scheduled actions in the reactors synchronized block, then there is a kind of lost-update problem. While Ractor.schedule() will call wakeup() on the selector, a thread could have already determined the value of selectWait, while being blocked at the start of the synchronized reactor section. Once it is able to enter the section, it will use an outdated selectWait value. This leads to scheduled actions not being executed on time. Thanks to Eng ChongMeng for reporting this and suggesting the fix. --- .../org/jivesoftware/smack/SmackReactor.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SmackReactor.java b/smack-core/src/main/java/org/jivesoftware/smack/SmackReactor.java index 230b0ea16..1c2477789 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SmackReactor.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackReactor.java @@ -204,25 +204,25 @@ public class SmackReactor { return; } - ScheduledAction nextScheduledAction = scheduledActions.peek(); - - long selectWait; - if (nextScheduledAction == null) { - // There is no next scheduled action, wait indefinitely in select() or until another thread invokes - // selector.wakeup(). - selectWait = 0; - } else { - selectWait = nextScheduledAction.getTimeToDueMillis(); - } - - if (selectWait < 0) { - // A scheduled action was just released and became ready to execute. - return; - } - int newSelectedKeysCount = 0; List selectedKeys; synchronized (selector) { + ScheduledAction nextScheduledAction = scheduledActions.peek(); + + long selectWait; + if (nextScheduledAction == null) { + // There is no next scheduled action, wait indefinitely in select() or until another thread invokes + // selector.wakeup(). + selectWait = 0; + } else { + selectWait = nextScheduledAction.getTimeToDueMillis(); + } + + if (selectWait < 0) { + // A scheduled action was just released and became ready to execute. + return; + } + // Before we call select, we handle the pending the interest Ops. This will not block since no other // thread is currently in select() at this time. // Note: This was put deliberately before the registration lock. It may cause more synchronization but