Remove LOCK in OmemoManager and use Manager instance instead

This commit is contained in:
Florian Schmaus 2019-08-06 08:23:10 +02:00
parent d7b7abc7eb
commit 9923268391
4 changed files with 103 additions and 132 deletions

View File

@ -55,6 +55,7 @@ public class MessageEncryptionIntegrationTest extends AbstractTwoUsersOmemoInteg
* Bob still has B2 * Bob still has B2
* @throws Exception * @throws Exception
*/ */
@SuppressWarnings("SynchronizeOnNonFinalField")
@SmackIntegrationTest @SmackIntegrationTest
public void messageTest() throws Exception { public void messageTest() throws Exception {
OmemoBundleElement a1 = alice.getOmemoService().getOmemoStoreBackend().packOmemoBundle(alice.getOwnDevice()); OmemoBundleElement a1 = alice.getOmemoService().getOmemoStoreBackend().packOmemoBundle(alice.getOwnDevice());
@ -74,7 +75,7 @@ public class MessageEncryptionIntegrationTest extends AbstractTwoUsersOmemoInteg
OmemoBundleElement a1_ = alice.getOmemoService().getOmemoStoreBackend().packOmemoBundle(alice.getOwnDevice()); OmemoBundleElement a1_ = alice.getOmemoService().getOmemoStoreBackend().packOmemoBundle(alice.getOwnDevice());
OmemoBundleElement b2; OmemoBundleElement b2;
synchronized (bob.LOCK) { // Circumvent race condition where bundle gets replenished after getting stored in b2 synchronized (bob) { // Circumvent race condition where bundle gets replenished after getting stored in b2
b2 = bob.getOmemoService().getOmemoStoreBackend().packOmemoBundle(bob.getOwnDevice()); b2 = bob.getOmemoService().getOmemoStoreBackend().packOmemoBundle(bob.getOwnDevice());
} }

View File

@ -31,6 +31,7 @@ public class SessionRenegotiationIntegrationTest extends AbstractTwoUsersOmemoIn
super(environment); super(environment);
} }
@SuppressWarnings("SynchronizeOnNonFinalField")
@SmackIntegrationTest @SmackIntegrationTest
public void sessionRenegotiationTest() throws Exception { public void sessionRenegotiationTest() throws Exception {
@ -50,7 +51,7 @@ public class SessionRenegotiationIntegrationTest extends AbstractTwoUsersOmemoIn
bob.removeOmemoMessageListener(listener1); bob.removeOmemoMessageListener(listener1);
// Remove the session on Bobs side. // Remove the session on Bobs side.
synchronized (bob.LOCK) { synchronized (bob) {
bob.getOmemoService().getOmemoStoreBackend().removeRawSession(bob.getOwnDevice(), alice.getOwnDevice()); bob.getOmemoService().getOmemoStoreBackend().removeRawSession(bob.getOwnDevice(), alice.getOwnDevice());
} }

View File

@ -97,7 +97,6 @@ public final class OmemoManager extends Manager {
private static final Logger LOGGER = Logger.getLogger(OmemoManager.class.getName()); private static final Logger LOGGER = Logger.getLogger(OmemoManager.class.getName());
private static final Integer UNKNOWN_DEVICE_ID = -1; private static final Integer UNKNOWN_DEVICE_ID = -1;
final Object LOCK = new Object();
private static final WeakHashMap<XMPPConnection, TreeMap<Integer, OmemoManager>> INSTANCES = new WeakHashMap<>(); private static final WeakHashMap<XMPPConnection, TreeMap<Integer, OmemoManager>> INSTANCES = new WeakHashMap<>();
private final OmemoService<?, ?, ?, ?, ?, ?, ?, ?, ?> service; private final OmemoService<?, ?, ?, ?, ?, ?, ?, ?, ?> service;
@ -237,11 +236,10 @@ public final class OmemoManager extends Manager {
* @throws PubSubException.NotALeafNodeException * @throws PubSubException.NotALeafNodeException
* @throws IOException * @throws IOException
*/ */
public void initialize() public synchronized void initialize()
throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, InterruptedException, throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, InterruptedException,
SmackException.NoResponseException, SmackException.NotConnectedException, XMPPException.XMPPErrorException, SmackException.NoResponseException, SmackException.NotConnectedException, XMPPException.XMPPErrorException,
PubSubException.NotALeafNodeException, IOException { PubSubException.NotALeafNodeException, IOException {
synchronized (LOCK) {
if (!connection().isAuthenticated()) { if (!connection().isAuthenticated()) {
throw new SmackException.NotLoggedInException(); throw new SmackException.NotLoggedInException();
} }
@ -253,7 +251,6 @@ public final class OmemoManager extends Manager {
getOmemoService().init(new LoggedInOmemoManager(this)); getOmemoService().init(new LoggedInOmemoManager(this));
ServiceDiscoveryManager.getInstanceFor(connection()).addFeature(PEP_NODE_DEVICE_LIST_NOTIFY); ServiceDiscoveryManager.getInstanceFor(connection()).addFeature(PEP_NODE_DEVICE_LIST_NOTIFY);
} }
}
/** /**
* Initialize the manager without blocking. Once the manager is successfully initialized, the finishedCallback will * Initialize the manager without blocking. Once the manager is successfully initialized, the finishedCallback will
@ -313,12 +310,10 @@ public final class OmemoManager extends Manager {
throws CryptoFailedException, UndecidedOmemoIdentityException, throws CryptoFailedException, UndecidedOmemoIdentityException,
InterruptedException, SmackException.NotConnectedException, InterruptedException, SmackException.NotConnectedException,
SmackException.NoResponseException, SmackException.NotLoggedInException, IOException { SmackException.NoResponseException, SmackException.NotLoggedInException, IOException {
synchronized (LOCK) {
Set<BareJid> recipients = new HashSet<>(); Set<BareJid> recipients = new HashSet<>();
recipients.add(recipient); recipients.add(recipient);
return encrypt(recipients, message); return encrypt(recipients, message);
} }
}
/** /**
* OMEMO encrypt a cleartext message for multiple recipients. * OMEMO encrypt a cleartext message for multiple recipients.
@ -334,11 +329,10 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NotLoggedInException * @throws SmackException.NotLoggedInException
* @throws IOException * @throws IOException
*/ */
public OmemoMessage.Sent encrypt(Set<BareJid> recipients, String message) public synchronized OmemoMessage.Sent encrypt(Set<BareJid> recipients, String message)
throws CryptoFailedException, UndecidedOmemoIdentityException, throws CryptoFailedException, UndecidedOmemoIdentityException,
InterruptedException, SmackException.NotConnectedException, InterruptedException, SmackException.NotConnectedException,
SmackException.NoResponseException, SmackException.NotLoggedInException, IOException { SmackException.NoResponseException, SmackException.NotLoggedInException, IOException {
synchronized (LOCK) {
LoggedInOmemoManager guard = new LoggedInOmemoManager(this); LoggedInOmemoManager guard = new LoggedInOmemoManager(this);
Set<OmemoDevice> devices = getDevicesOf(getOwnJid()); Set<OmemoDevice> devices = getDevicesOf(getOwnJid());
for (BareJid recipient : recipients) { for (BareJid recipient : recipients) {
@ -346,7 +340,6 @@ public final class OmemoManager extends Manager {
} }
return service.createOmemoMessage(guard, devices, message); return service.createOmemoMessage(guard, devices, message);
} }
}
/** /**
* Encrypt a message for all recipients in the MultiUserChat. * Encrypt a message for all recipients in the MultiUserChat.
@ -364,12 +357,11 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NotLoggedInException * @throws SmackException.NotLoggedInException
* @throws IOException * @throws IOException
*/ */
public OmemoMessage.Sent encrypt(MultiUserChat muc, String message) public synchronized OmemoMessage.Sent encrypt(MultiUserChat muc, String message)
throws UndecidedOmemoIdentityException, CryptoFailedException, throws UndecidedOmemoIdentityException, CryptoFailedException,
XMPPException.XMPPErrorException, SmackException.NotConnectedException, InterruptedException, XMPPException.XMPPErrorException, SmackException.NotConnectedException, InterruptedException,
SmackException.NoResponseException, NoOmemoSupportException, SmackException.NoResponseException, NoOmemoSupportException,
SmackException.NotLoggedInException, IOException { SmackException.NotLoggedInException, IOException {
synchronized (LOCK) {
if (!multiUserChatSupportsOmemo(muc)) { if (!multiUserChatSupportsOmemo(muc)) {
throw new NoOmemoSupportException(); throw new NoOmemoSupportException();
} }
@ -381,7 +373,6 @@ public final class OmemoManager extends Manager {
} }
return encrypt(recipients, message); return encrypt(recipients, message);
} }
}
/** /**
* Manually decrypt an OmemoElement. * Manually decrypt an OmemoElement.
@ -496,24 +487,21 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NotConnectedException * @throws SmackException.NotConnectedException
* @throws IOException * @throws IOException
*/ */
public void sendRatchetUpdateMessage(OmemoDevice recipient) public synchronized void sendRatchetUpdateMessage(OmemoDevice recipient)
throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, InterruptedException, throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, InterruptedException,
SmackException.NoResponseException, NoSuchAlgorithmException, SmackException.NotConnectedException, SmackException.NoResponseException, NoSuchAlgorithmException, SmackException.NotConnectedException,
CryptoFailedException, CannotEstablishOmemoSessionException, IOException { CryptoFailedException, CannotEstablishOmemoSessionException, IOException {
synchronized (LOCK) {
Message message = new Message(); Message message = new Message();
message.setFrom(getOwnJid()); message.setFrom(getOwnJid());
message.setTo(recipient.getJid()); message.setTo(recipient.getJid());
OmemoElement element = getOmemoService() OmemoElement element = getOmemoService().createRatchetUpdateElement(new LoggedInOmemoManager(this), recipient);
.createRatchetUpdateElement(new LoggedInOmemoManager(this), recipient);
message.addExtension(element); message.addExtension(element);
// Set MAM Storage hint // Set MAM Storage hint
StoreHint.set(message); StoreHint.set(message);
connection().sendStanza(message); connection().sendStanza(message);
} }
}
/** /**
* Returns true, if the contact has any active devices published in a deviceList. * Returns true, if the contact has any active devices published in a deviceList.
@ -527,14 +515,12 @@ public final class OmemoManager extends Manager {
* @throws XMPPException.XMPPErrorException * @throws XMPPException.XMPPErrorException
* @throws IOException * @throws IOException
*/ */
public boolean contactSupportsOmemo(BareJid contact) public synchronized boolean contactSupportsOmemo(BareJid contact)
throws InterruptedException, PubSubException.NotALeafNodeException, XMPPException.XMPPErrorException, throws InterruptedException, PubSubException.NotALeafNodeException, XMPPException.XMPPErrorException,
SmackException.NotConnectedException, SmackException.NoResponseException, IOException { SmackException.NotConnectedException, SmackException.NoResponseException, IOException {
synchronized (LOCK) {
OmemoCachedDeviceList deviceList = getOmemoService().refreshDeviceList(connection(), getOwnDevice(), contact); OmemoCachedDeviceList deviceList = getOmemoService().refreshDeviceList(connection(), getOwnDevice(), contact);
return !deviceList.getActiveDevices().isEmpty(); return !deviceList.getActiveDevices().isEmpty();
} }
}
/** /**
* Returns true, if the MUC with the EntityBareJid multiUserChat is non-anonymous and members only (prerequisite * Returns true, if the MUC with the EntityBareJid multiUserChat is non-anonymous and members only (prerequisite
@ -581,16 +567,14 @@ public final class OmemoManager extends Manager {
* @throws CorruptedOmemoKeyException if our identityKey is corrupted. * @throws CorruptedOmemoKeyException if our identityKey is corrupted.
* @throws IOException * @throws IOException
*/ */
public OmemoFingerprint getOwnFingerprint() public synchronized OmemoFingerprint getOwnFingerprint()
throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, IOException { throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, IOException {
synchronized (LOCK) {
if (getOwnJid() == null) { if (getOwnJid() == null) {
throw new SmackException.NotLoggedInException(); throw new SmackException.NotLoggedInException();
} }
return getOmemoService().getOmemoStoreBackend().getFingerprint(getOwnDevice()); return getOmemoService().getOmemoStoreBackend().getFingerprint(getOwnDevice());
} }
}
/** /**
* Get the fingerprint of a contacts device. * Get the fingerprint of a contacts device.
@ -604,11 +588,10 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NoResponseException * @throws SmackException.NoResponseException
* @throws IOException * @throws IOException
*/ */
public OmemoFingerprint getFingerprint(OmemoDevice device) public synchronized OmemoFingerprint getFingerprint(OmemoDevice device)
throws CannotEstablishOmemoSessionException, SmackException.NotLoggedInException, throws CannotEstablishOmemoSessionException, SmackException.NotLoggedInException,
CorruptedOmemoKeyException, SmackException.NotConnectedException, InterruptedException, CorruptedOmemoKeyException, SmackException.NotConnectedException, InterruptedException,
SmackException.NoResponseException, IOException { SmackException.NoResponseException, IOException {
synchronized (LOCK) {
if (getOwnJid() == null) { if (getOwnJid() == null) {
throw new SmackException.NotLoggedInException(); throw new SmackException.NotLoggedInException();
} }
@ -617,8 +600,8 @@ public final class OmemoManager extends Manager {
return getOwnFingerprint(); return getOwnFingerprint();
} }
return getOmemoService().getOmemoStoreBackend().getFingerprintAndMaybeBuildSession(new LoggedInOmemoManager(this), device); return getOmemoService().getOmemoStoreBackend()
} .getFingerprintAndMaybeBuildSession(new LoggedInOmemoManager(this), device);
} }
/** /**
@ -635,18 +618,17 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NoResponseException * @throws SmackException.NoResponseException
* @throws IOException * @throws IOException
*/ */
public HashMap<OmemoDevice, OmemoFingerprint> getActiveFingerprints(BareJid contact) public synchronized HashMap<OmemoDevice, OmemoFingerprint> getActiveFingerprints(BareJid contact)
throws SmackException.NotLoggedInException, CorruptedOmemoKeyException, throws SmackException.NotLoggedInException, CorruptedOmemoKeyException,
CannotEstablishOmemoSessionException, SmackException.NotConnectedException, InterruptedException, CannotEstablishOmemoSessionException, SmackException.NotConnectedException, InterruptedException,
SmackException.NoResponseException, IOException { SmackException.NoResponseException, IOException {
synchronized (LOCK) {
if (getOwnJid() == null) { if (getOwnJid() == null) {
throw new SmackException.NotLoggedInException(); throw new SmackException.NotLoggedInException();
} }
HashMap<OmemoDevice, OmemoFingerprint> fingerprints = new HashMap<>(); HashMap<OmemoDevice, OmemoFingerprint> fingerprints = new HashMap<>();
OmemoCachedDeviceList deviceList = getOmemoService().getOmemoStoreBackend() OmemoCachedDeviceList deviceList = getOmemoService().getOmemoStoreBackend().loadCachedDeviceList(getOwnDevice(),
.loadCachedDeviceList(getOwnDevice(), contact); contact);
for (int id : deviceList.getActiveDevices()) { for (int id : deviceList.getActiveDevices()) {
OmemoDevice device = new OmemoDevice(contact, id); OmemoDevice device = new OmemoDevice(contact, id);
@ -659,7 +641,6 @@ public final class OmemoManager extends Manager {
return fingerprints; return fingerprints;
} }
}
/** /**
* Add an OmemoMessageListener. This listener will be informed about incoming OMEMO messages * Add an OmemoMessageListener. This listener will be informed about incoming OMEMO messages
@ -707,13 +688,11 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NoResponseException * @throws SmackException.NoResponseException
* @throws IOException * @throws IOException
*/ */
public void requestDeviceListUpdateFor(BareJid contact) public synchronized void requestDeviceListUpdateFor(BareJid contact)
throws InterruptedException, PubSubException.NotALeafNodeException, XMPPException.XMPPErrorException, throws InterruptedException, PubSubException.NotALeafNodeException, XMPPException.XMPPErrorException,
SmackException.NotConnectedException, SmackException.NoResponseException, IOException { SmackException.NotConnectedException, SmackException.NoResponseException, IOException {
synchronized (LOCK) {
getOmemoService().refreshDeviceList(connection(), getOwnDevice(), contact); getOmemoService().refreshDeviceList(connection(), getOwnDevice(), contact);
} }
}
/** /**
* Publish a new device list with just our own deviceId in it. * Publish a new device list with just our own deviceId in it.
@ -728,10 +707,8 @@ public final class OmemoManager extends Manager {
public void purgeDeviceList() public void purgeDeviceList()
throws SmackException.NotLoggedInException, InterruptedException, XMPPException.XMPPErrorException, throws SmackException.NotLoggedInException, InterruptedException, XMPPException.XMPPErrorException,
SmackException.NotConnectedException, SmackException.NoResponseException, IOException { SmackException.NotConnectedException, SmackException.NoResponseException, IOException {
synchronized (LOCK) {
getOmemoService().purgeDeviceList(new LoggedInOmemoManager(this)); getOmemoService().purgeDeviceList(new LoggedInOmemoManager(this));
} }
}
/** /**
* Rotate the signedPreKey published in our OmemoBundle and republish it. This should be done every now and * Rotate the signedPreKey published in our OmemoBundle and republish it. This should be done every now and
@ -746,10 +723,9 @@ public final class OmemoManager extends Manager {
* @throws SmackException.NotLoggedInException * @throws SmackException.NotLoggedInException
* @throws IOException * @throws IOException
*/ */
public void rotateSignedPreKey() public synchronized void rotateSignedPreKey()
throws CorruptedOmemoKeyException, SmackException.NotLoggedInException, XMPPException.XMPPErrorException, throws CorruptedOmemoKeyException, SmackException.NotLoggedInException, XMPPException.XMPPErrorException,
SmackException.NotConnectedException, InterruptedException, SmackException.NoResponseException, IOException { SmackException.NotConnectedException, InterruptedException, SmackException.NoResponseException, IOException {
synchronized (LOCK) {
if (!connection().isAuthenticated()) { if (!connection().isAuthenticated()) {
throw new SmackException.NotLoggedInException(); throw new SmackException.NotLoggedInException();
} }
@ -761,7 +737,6 @@ public final class OmemoManager extends Manager {
OmemoBundleElement bundle = getOmemoService().getOmemoStoreBackend().packOmemoBundle(getOwnDevice()); OmemoBundleElement bundle = getOmemoService().getOmemoStoreBackend().packOmemoBundle(getOwnDevice());
OmemoService.publishBundle(connection(), getOwnDevice(), bundle); OmemoService.publishBundle(connection(), getOwnDevice(), bundle);
} }
}
/** /**
* Return true, if the given Stanza contains an OMEMO element 'encrypted'. * Return true, if the given Stanza contains an OMEMO element 'encrypted'.
@ -807,40 +782,34 @@ public final class OmemoManager extends Manager {
* *
* @return deviceId * @return deviceId
*/ */
public Integer getDeviceId() { public synchronized Integer getDeviceId() {
synchronized (LOCK) {
return deviceId; return deviceId;
} }
}
/** /**
* Return the OmemoDevice of the user. * Return the OmemoDevice of the user.
* *
* @return omemoDevice * @return omemoDevice
*/ */
public OmemoDevice getOwnDevice() { public synchronized OmemoDevice getOwnDevice() {
synchronized (LOCK) {
BareJid jid = getOwnJid(); BareJid jid = getOwnJid();
if (jid == null) { if (jid == null) {
return null; return null;
} }
return new OmemoDevice(jid, getDeviceId()); return new OmemoDevice(jid, getDeviceId());
} }
}
/** /**
* Set the deviceId of the manager to nDeviceId. * Set the deviceId of the manager to nDeviceId.
* @param nDeviceId new deviceId * @param nDeviceId new deviceId
*/ */
void setDeviceId(int nDeviceId) { synchronized void setDeviceId(int nDeviceId) {
synchronized (LOCK) {
// Move this instance inside the HashMaps // Move this instance inside the HashMaps
INSTANCES.get(connection()).remove(getDeviceId()); INSTANCES.get(connection()).remove(getDeviceId());
INSTANCES.get(connection()).put(nDeviceId, this); INSTANCES.get(connection()).put(nDeviceId, this);
this.deviceId = nDeviceId; this.deviceId = nDeviceId;
} }
}
/** /**
* Notify all registered OmemoMessageListeners about a received OmemoMessage. * Notify all registered OmemoMessageListeners about a received OmemoMessage.

View File

@ -1075,7 +1075,7 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
OmemoManager.LoggedInOmemoManager managerGuard) throws IOException { OmemoManager.LoggedInOmemoManager managerGuard) throws IOException {
OmemoManager manager = managerGuard.get(); OmemoManager manager = managerGuard.get();
// Avoid the ratchet being manipulated and the bundle being published multiple times simultaneously // Avoid the ratchet being manipulated and the bundle being published multiple times simultaneously
synchronized (manager.LOCK) { synchronized (manager) {
OmemoDevice userDevice = manager.getOwnDevice(); OmemoDevice userDevice = manager.getOwnDevice();
OmemoElement element = carbonCopy.getExtension(OmemoElement.NAME_ENCRYPTED, OmemoElement_VAxolotl.NAMESPACE); OmemoElement element = carbonCopy.getExtension(OmemoElement.NAME_ENCRYPTED, OmemoElement_VAxolotl.NAMESPACE);
if (element == null) { if (element == null) {
@ -1130,7 +1130,7 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
public void onOmemoMessageStanzaReceived(Stanza stanza, OmemoManager.LoggedInOmemoManager managerGuard) throws IOException { public void onOmemoMessageStanzaReceived(Stanza stanza, OmemoManager.LoggedInOmemoManager managerGuard) throws IOException {
OmemoManager manager = managerGuard.get(); OmemoManager manager = managerGuard.get();
// Avoid the ratchet being manipulated and the bundle being published multiple times simultaneously // Avoid the ratchet being manipulated and the bundle being published multiple times simultaneously
synchronized (manager.LOCK) { synchronized (manager) {
OmemoDevice userDevice = manager.getOwnDevice(); OmemoDevice userDevice = manager.getOwnDevice();
OmemoElement element = stanza.getExtension(OmemoElement.NAME_ENCRYPTED, OmemoElement_VAxolotl.NAMESPACE); OmemoElement element = stanza.getExtension(OmemoElement.NAME_ENCRYPTED, OmemoElement_VAxolotl.NAMESPACE);
if (element == null) { if (element == null) {
@ -1219,7 +1219,7 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
OmemoMessage.Received decryptStanza(Stanza stanza, OmemoManager.LoggedInOmemoManager managerGuard) throws IOException { OmemoMessage.Received decryptStanza(Stanza stanza, OmemoManager.LoggedInOmemoManager managerGuard) throws IOException {
OmemoManager manager = managerGuard.get(); OmemoManager manager = managerGuard.get();
// Avoid the ratchet being manipulated and the bundle being published multiple times simultaneously // Avoid the ratchet being manipulated and the bundle being published multiple times simultaneously
synchronized (manager.LOCK) { synchronized (manager) {
OmemoDevice userDevice = manager.getOwnDevice(); OmemoDevice userDevice = manager.getOwnDevice();
OmemoElement element = stanza.getExtension(OmemoElement.NAME_ENCRYPTED, OmemoElement_VAxolotl.NAMESPACE); OmemoElement element = stanza.getExtension(OmemoElement.NAME_ENCRYPTED, OmemoElement_VAxolotl.NAMESPACE);
if (element == null) { if (element == null) {