Skip to content

Commit 60f9347

Browse files
committed
OF-3155: Fix race condition in resource conflict resolution
The logic that handles resource conflicts closed the old session asynchronously while allowing the new session to be marked as bound immediately. This could cause the new session to be affected by cleanup intended for the old session, such as receiving the old session's unavailable presence. Internal state could also become inconsistent. This update introduces a temporary wait mechanism that delays promotion of the new session until the old session has fully closed. A busy-wait loop is used as a stopgap, as connectionClosed event listeners are not reliable in clustered deployments and do not guarantee that all listeners have completed execution. This prevents the new session from being influenced by asynchronous cleanup of the old session and avoids related routing and presence issues. (cherry picked from commit 50be000)
1 parent b909e18 commit 60f9347

1 file changed

Lines changed: 12 additions & 0 deletions

File tree

xmppserver/src/main/java/org/jivesoftware/openfire/handler/IQBindHandler.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@
3636
import org.xmpp.packet.PacketError;
3737
import org.xmpp.packet.StreamError;
3838

39+
import java.time.Duration;
40+
import java.time.Instant;
41+
3942
/**
4043
* Binds a resource to the stream so that the client's address becomes a full JID. Once a resource
4144
* has been bound to the session the entity (i.e. client) is considered a "connected resource".
@@ -151,6 +154,15 @@ public IQ handleIQ(IQ packet) throws UnauthorizedException {
151154
// the old session again, as that would cause the state of the new session to be affected.
152155
sessionManager.removeDetached((LocalClientSession) oldSession);
153156
}
157+
158+
// OF-3155: Await closure of the old session, to prevent the _new_ session to be influenced by the closure process.
159+
final Instant deadline = Instant.now().plus(Duration.ofSeconds(20));
160+
do {
161+
// TODO find a better way to wait for closure than using a busy-wait mechanism. Using the connectionClosed event listener
162+
// a) doesn't work in a cluster and
163+
// b) doesn't reliably give an indication that all _other_ connectionClosed listeners have been called, which could still cause issues.
164+
Thread.sleep(Duration.ofMillis(50).toMillis());
165+
} while (Instant.now().isBefore(deadline) && routingTable.getClientRoute(desiredJid) != null);
154166
} else {
155167
Log.debug("Conflict resolution configuration does not allow kicking of old session (yet). Conflict count: {}, conflict limit: {}. Rejecting the bind request with error condition 'conflict'.", conflictCount, conflictLimit);
156168
reply.setChildElement(packet.getChildElement().createCopy());

0 commit comments

Comments
 (0)