powered by Jive Software

Smack 4.4.0-alpha3 SmackReactor does not warrant the interval time set by PingManager

Please refer to the following links for more info:

Smack 4.4.0-alpha2 SmackReactor reactorLoop die unexpectedly causing PingManager not working

Smack 4.4.0 Synchronized (selector) access in SmackReactor class is holding back normal process execution; leading to android OS “Long monitor contention”

Java NIO Selector

It is found that smack 4.4.0-alpha3 SmackReactor does not adhere to the period set by the PingManager in most of the times. To make SmackReactor to properly execute ping interval as defined by the PingManager, aTalk has applied the following patch i.e.

SmackReactor2.patch (2.3 KB)

Following is an attempt to explain the SmackReactor behavior and the implication of the applied patch.

The method handleScheduledActionsOrPerformSelect() is being called/triggered from two locations i.e.
a. In reactorLoop() in a continuous loop until (shutdownRequestTimestamp >= 0)

        private void reactorLoop() {
            // Loop until reactor shutdown was requested.
            while (shutdownRequestTimestamp < 0) {
                handleScheduledActionsOrPerformSelect();

                handlePendingSelectionKeys();
            }
        }

b. via selector.wakeup in schedule() method.

    ScheduledAction schedule(Runnable runnable, long delay, TimeUnit unit) {
        long releaseTimeEpoch = System.currentTimeMillis() + unit.toMillis(delay);
        Date releaseTimeDate = new Date(releaseTimeEpoch);
        ScheduledAction scheduledAction = new ScheduledAction(runnable, releaseTimeDate, this);
        scheduledActions.add(scheduledAction);
        selector.wakeup();
        return scheduledAction;
    }

Both callings are not synchronous; and there are also differences in which part of code are being executed during each call from observation.

It is observed that after the selector.select(selectWait) is being performed, the selector is in the blocking mode as explained in the javadoc below (may be this is the reason why leading to android OS “Long monitor contention” warning). It seems that the selector.wakeup() in (b) only causes the system to perform code execution within the synchronized (selector) { } block.

 * <p> This method performs a blocking <a href="#selop">selection
 * operation</a>.  It returns only after at least one channel is selected,
 * this selector's {@link #wakeup wakeup} method is invoked, the current
 * thread is interrupted, or the given timeout period expires, whichever
 * comes first.

It is also found that the selectWait still contains a local value that was used in selector.select(selectWait) in the previous synchronized (selector) { } execution before the blocking process, i.e. it does not get updated with the newly schedule() delay time. In order for the wakeup synchronized (selector) { } to use the new delay time, the above mentioned patch must be applied i.e. to get the synchronized (selector) to include/update its nextScheduledAction = scheduledActions.peek() and hence its selectWait.

when the call is made via (a), it executes the whole handleScheduledActionsOrPerformSelect() from start until synchronized (selector) { } and skip over if is still blocked by the previous selector.select(selectWait). Therefore even it has the newly added nextScheduledAction, is is unable to pass into the blocked synchronized (selector) { }.

Thanks, good work! :slight_smile:

I have pushed

Please test and report back if it fixes the issue for you.

After some thought, I think there was some mistake in my earlier explanation in my last two paragraphs which I believe belows are more accurate:

On return from the selector.select(selectWait), the while loop in (a) will cause it to immediately execute handleScheduledActionsOrPerformSelect; it executes from start until synchronized (selector) { }, then wait for the selector to be released.

At this point the selectWait is likely to contain the same value that was used in selector.select(selectWait) in the previous synchronized (selector) { } execution before the blocking process. This old value gets pass in to the synchronized (selector) { } when the next selector.wake() is called. i.e. it does not get updated with the newly schedule() delay time. In order for the wakeup synchronized (selector) { } to use the new delay time, the above mentioned patch must be applied i.e. to get the synchronized (selector) to include/update its nextScheduledAction = scheduledActions.peek() and hence its selectWait.