vanitasvitae/Smack
1
0
Fork 0
mirror of https://github.com/vanitasvitae/Smack.git synced 2024-11-25 13:32:07 +01:00
Code Issues Releases Wiki Activity

[muc] Fix MultiUserChat.changeNickname(Resourcepart)

Browse source 
Using this method used to result in a deadlock, as shown by these two threads

"main" #1 prio=5 os_prio=0 cpu=926.39ms elapsed=21.00s tid=0x00007f463802c800 nid=0x5a691 in Object.wait()  [0x00007f463f323000]
   java.lang.Thread.State: TIMED_WAITING (on object monitor)
	at java.lang.Object.wait(java.base@11.0.23/Native Method)
	- waiting on <0x0000000622e82fd8> (a org.jivesoftware.smack.StanzaCollector)
	at org.jivesoftware.smack.StanzaCollector.nextResult(StanzaCollector.java:206)
	- waiting to re-lock in wait() <0x0000000622e82fd8> (a org.jivesoftware.smack.StanzaCollector)
	at org.jivesoftware.smack.StanzaCollector.nextResultOrThrow(StanzaCollector.java:270)
	at org.jivesoftware.smack.StanzaCollector.nextResultOrThrow(StanzaCollector.java:228)
	at org.jivesoftware.smackx.muc.MultiUserChat.changeNickname(MultiUserChat.java:1314)
	- locked <0x0000000622e19700> (a org.jivesoftware.smackx.muc.MultiUserChat)
	at org.jivesoftware.smackx.muc.MultiUserChatOccupantIntegrationTest.mucChangeNicknameInformationTest(MultiUserChatOccupantIntegrationTest.java:981)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@11.0.23/Native Method)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@11.0.23/NativeMethodAccessorImpl.java:62)
	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@11.0.23/DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(java.base@11.0.23/Method.java:566)
	at org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.lambda$runTests$0(SmackIntegrationTestFramework.java:476)
	at org.igniterealtime.smack.inttest.SmackIntegrationTestFramework$$Lambda$141/0x000000084026e040.execute(Unknown Source)
	at org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.runConcreteTest(SmackIntegrationTestFramework.java:551)
	at org.igniterealtime.smack.inttest.SmackIntegrationTestFramework$PreparedTest.run(SmackIntegrationTestFramework.java:759)
	at org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.runTests(SmackIntegrationTestFramework.java:539)
	at org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.run(SmackIntegrationTestFramework.java:277)
	- locked <0x000000062d191318> (a org.igniterealtime.smack.inttest.SmackIntegrationTestFramework)
	at
	org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.main(SmackIntegrationTestFramework.java:115)

"Smack Cached Executor" #19 daemon prio=5 os_prio=0 cpu=7.85ms elapsed=20.48s tid=0x00007f4638a42800 nid=0x5a6b2 waiting for monitor entry  [0x00007f46023fe000]
   java.lang.Thread.State: BLOCKED (on object monitor)
	at org.jivesoftware.smackx.muc.MultiUserChat.userHasLeft(MultiUserChat.java:2281)
	- waiting to lock <0x0000000622e19700> (a org.jivesoftware.smackx.muc.MultiUserChat)
	at org.jivesoftware.smackx.muc.MultiUserChat.access$800(MultiUserChat.java:117)
	at org.jivesoftware.smackx.muc.MultiUserChat$3.processStanza(MultiUserChat.java:263)
	at org.jivesoftware.smack.AbstractXMPPConnection.lambda$invokeStanzaCollectorsAndNotifyRecvListeners$8(AbstractXMPPConnection.java:1654)
	at org.jivesoftware.smack.AbstractXMPPConnection$$Lambda$127/0x000000084022f440.run(Unknown Source)
	at org.jivesoftware.smack.AbstractXMPPConnection$10.run(AbstractXMPPConnection.java:2213)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@11.0.23/ThreadPoolExecutor.java:1128)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@11.0.23/ThreadPoolExecutor.java:628)
	at java.lang.Thread.run(java.base@11.0.23/Thread.java:829)

The changeNickname() method was synchronized and is userHasLeft(),
this caused a deadlock since changeNickname() awaits the presence that
is send as result of the nickname change. But this presence is also
processed in a listener which invokes userHasLeft(). However, this
invocation blocks as userHasLeft() is waiting on the montior currently
hold by the thread invoking changeNickname().

To fix this, we change changeNickname() to not take the MultiUserChat
monitor, but instead use a dedicate lock.

Fixes SMACK-942.
This commit is contained in:
Florian Schmaus 2024-05-20 21:38:18 +02:00
parent 1836537c42
commit c34c9bdb62
1 changed files with 17 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java
View file

@ -1,6 +1,6 @@
/** /**
* *
* Copyright 2003-2007 Jive Software. 2020-2022 Florian Schmaus * Copyright 2003-2007 Jive Software. 2020-2024 Florian Schmaus
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -1273,6 +1273,8 @@ public class MultiUserChat {
return myRoomJid; return myRoomJid;
} }
private static final Object changeNicknameLock = new Object();
/** /**
* Changes the occupant's nickname to a new nickname within the room. Each room occupant * Changes the occupant's nickname to a new nickname within the room. Each room occupant
* will receive two presence packets. One of type "unavailable" for the old nickname and one * will receive two presence packets. One of type "unavailable" for the old nickname and one
@ -1287,7 +1289,7 @@ public class MultiUserChat {
* @throws InterruptedException if the calling thread was interrupted. * @throws InterruptedException if the calling thread was interrupted.
* @throws MucNotJoinedException if not joined to the Multi-User Chat. * @throws MucNotJoinedException if not joined to the Multi-User Chat.
*/ */
public synchronized void changeNickname(Resourcepart nickname) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException, MucNotJoinedException { public void changeNickname(Resourcepart nickname) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException, MucNotJoinedException {
Objects.requireNonNull(nickname, "Nickname must not be null or blank."); Objects.requireNonNull(nickname, "Nickname must not be null or blank.");
// Check that we already have joined the room before attempting to change the // Check that we already have joined the room before attempting to change the
// nickname. // nickname.
@ -1303,18 +1305,20 @@ public class MultiUserChat {
.ofType(Presence.Type.available) .ofType(Presence.Type.available)
.build(); .build();
// Wait for a presence packet back from the server. synchronized (changeNicknameLock) {
StanzaFilter responseFilter = // Wait for a presence packet back from the server.
new AndFilter( StanzaFilter responseFilter =
FromMatchesFilter.createFull(jid), new AndFilter(
new StanzaTypeFilter(Presence.class)); FromMatchesFilter.createFull(jid),
StanzaCollector response = connection.createStanzaCollectorAndSend(responseFilter, joinPresence); new StanzaTypeFilter(Presence.class));
// Wait up to a certain number of seconds for a reply. If there is a negative reply, an StanzaCollector response = connection.createStanzaCollectorAndSend(responseFilter, joinPresence);
// exception will be thrown // Wait up to a certain number of seconds for a reply. If there is a negative reply, an
response.nextResultOrThrow(); // exception will be thrown
response.nextResultOrThrow();
// TODO: Shouldn't this handle nickname rewriting by the MUC service? // TODO: Shouldn't this handle nickname rewriting by the MUC service?
setNickname(nickname); setNickname(nickname);
}
} }
/** /**