From b1a4ccfae8999b900947029ad72f819fa2918328 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 30 May 2020 19:37:48 +0200 Subject: [PATCH] [core] Do not weakly reference "channel selected" callback Since d65f2c932 ("Bump Error Prone version to 2.3.4 and fix new bug patterns") the channel selected callback is no longer a final field of the connection instance, hence it may be come null even if the connection instance is still strongly referenced. Also the ConnectionAttemptState class uses simply a lambda as callback, which is also not strongly referenced otherwise. The "channel selected" callback was wrapped in weak reference, so that connection instances could get gc'ed if they are still connected but the user lost all references to them. In this case, the weak reference to the connection instance would become 'null' and selectionKey.cancel() would be called. This change means that a socket and its selection key of a "leaked" connected connection instance continues to be part of the reactor. But this may not be that bad: first, users are expected to manager their connection instances, and disconnect them before they are discarded. And secondly, at some point the connection likely will get disconnected, and in this case, the socket and its selection key will be removed from the reactor. --- .../java/org/jivesoftware/smack/SmackReactor.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 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 eaff9ca3d..75e1af8e9 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SmackReactor.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackReactor.java @@ -17,7 +17,6 @@ package org.jivesoftware.smack; import java.io.IOException; -import java.lang.ref.WeakReference; import java.nio.channels.CancelledKeyException; import java.nio.channels.ClosedChannelException; import java.nio.channels.SelectableChannel; @@ -368,13 +367,8 @@ public class SmackReactor { for (SelectionKey selectionKey : selectedKeys) { SelectableChannel channel = selectionKey.channel(); SelectionKeyAttachment selectionKeyAttachment = (SelectionKeyAttachment) selectionKey.attachment(); - ChannelSelectedCallback channelSelectedCallback = selectionKeyAttachment.weaeklyReferencedChannelSelectedCallback.get(); - if (channelSelectedCallback != null) { - channelSelectedCallback.onChannelSelected(channel, selectionKey); - } - else { - selectionKey.cancel(); - } + ChannelSelectedCallback channelSelectedCallback = selectionKeyAttachment.channelSelectedCallback; + channelSelectedCallback.onChannelSelected(channel, selectionKey); } } @@ -422,11 +416,11 @@ public class SmackReactor { } public static final class SelectionKeyAttachment { - private final WeakReference weaeklyReferencedChannelSelectedCallback; + private final ChannelSelectedCallback channelSelectedCallback; private final AtomicBoolean reactorThreadRacing = new AtomicBoolean(); private SelectionKeyAttachment(ChannelSelectedCallback channelSelectedCallback) { - this.weaeklyReferencedChannelSelectedCallback = new WeakReference<>(channelSelectedCallback); + this.channelSelectedCallback = channelSelectedCallback; } private void setRacing() {