Cautiously declaring victory.

Igniterealtime’s Openfire is now dogfooding a change referencing this ticket to see if the deadlocks are gone.

One thing that’s a potential source of problems: when a stanza gets ‘bounced’ by the S2S implementation, the processing of the bounce is processed inline with the original code (meaning that all locks held by the S2S processing remain in place). This arguably isn’t correct, as processing of the bounce shouldn’t need to be under S2S lock (unlike the original stanza that they’re a bounce for, these bounces are not so much ‘outgoing’ stanzas).

I’m seeing the same mechanism in a new deadlock, described on .

pool-5-thread-84 is busy removing a user from all chat rooms. It seems to do so in reaction to a 'bounced stanza'. That stanza was bounced by LocalOutgoingServerSession, as it could not send process/deliver the original stanza. Processing of the bounced stanza is blocked in the routine that tries to obtain a lock on a room from which to remove a the user.

The bounced stanza seems to be this one (I've formatted the stanza for readability):

pool-5-thread-84 seems to have already attempted to remove user 'guus.der.kinderen' from chat room 'smack':

pool-5-thread-84 next tries to remove the same user from room 'open_chat':

That's where pool-5-thread-84 gets 'stuck'. The thread remains on the location where it is attempting to obtain that lock, as shown in this stack trace:

The corresponding code snippet from MultiUserChatServiceImpl.removeChatUser:

It should be noted that pool-5-thread-84 holds a lock on DomainPair while attempting to remove the chat user.

Side-note: I can't immediately see a reason why the stanza that caused this bounce was processed by LocalOutgoingServerSession in the first place, as the addressee (guus.der.kinderen at ignite) is a local user. This should be the subject of further analysis.

The other thread, TaskEngine-pool-752, is broadcasting a chatroom leave presence, triggered by a session having been closed.
Processing is blocked in the routine that broadcasts the corresponding presence update to a remote domain.

Note that the addressing of the stanza in the last line seems off. The intended recipient is likely not yet modified to be 'holger' when this line is logged. This is probably fine, just a quirk of the way things are logged.

That's where TaskEngine-pool-752 gets 'stuck'. The thread remains on the location where it is attempting to obtain a lock for a DomainPair, representing the remote domain of holger, presumably, as shown in this stack trace:

What I do not get: why does thread pool-5-thread-84 hold a lock to the DomainPair for Holger's server? Is that the original outgoing session that caused a bounce?

Lock objects themselves do not do anything. Calling the lock() method on them is where things start to get interesting. However, this action should be ‘re-entrant’, meaning that when a thread has already acquired the mutex on the lock, it can immediately do so (successfully) again.

I do not think this is a problem. The problem here is that we have two flows that are in odds with each-other:

• One flow first acquires lock A, does some stuff, and then wants to acquire lock B (while still holding lock A).

• Another flow first acquires lock B, then does some other stuff, and then wants to acquire lock A (while still holding lock B.)

This results in a situation where both flows (threads) are waiting indefinitely on each-other.

I do not expect that this is caused by a bug in the locking mechanism itself. Instead, I suspect that this is a bug in how we utilize them. Ideally, we should prevent a scenario where two different locks can be obtained by flows in different orders.