Fix potential dead lock in recipient store

This commit is contained in:
AsamK 2024-02-09 17:51:15 +01:00
parent 4e61f2b2e5
commit be699cbd85

View file

@ -47,7 +47,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
private final SelfProfileKeyProvider selfProfileKeyProvider; private final SelfProfileKeyProvider selfProfileKeyProvider;
private final Database database; private final Database database;
private final Object recipientsLock = new Object();
private final Map<Long, Long> recipientsMerged = new HashMap<>(); private final Map<Long, Long> recipientsMerged = new HashMap<>();
private final Map<ServiceId, RecipientWithAddress> recipientAddressCache = new HashMap<>(); private final Map<ServiceId, RecipientWithAddress> recipientAddressCache = new HashMap<>();
@ -164,7 +163,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} }
private RecipientId resolveRecipientByNumber(final String number) { private RecipientId resolveRecipientByNumber(final String number) {
synchronized (recipientsLock) {
final RecipientId recipientId; final RecipientId recipientId;
try (final var connection = database.getConnection()) { try (final var connection = database.getConnection()) {
connection.setAutoCommit(false); connection.setAutoCommit(false);
@ -175,17 +173,15 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} }
return recipientId; return recipientId;
} }
}
@Override @Override
public RecipientId resolveRecipient(final ServiceId serviceId) { public RecipientId resolveRecipient(final ServiceId serviceId) {
synchronized (recipientsLock) { try (final var connection = database.getConnection()) {
connection.setAutoCommit(false);
final var recipientWithAddress = recipientAddressCache.get(serviceId); final var recipientWithAddress = recipientAddressCache.get(serviceId);
if (recipientWithAddress != null) { if (recipientWithAddress != null) {
return recipientWithAddress.id(); return recipientWithAddress.id();
} }
try (final var connection = database.getConnection()) {
connection.setAutoCommit(false);
final var recipientId = resolveRecipientLocked(connection, serviceId); final var recipientId = resolveRecipientLocked(connection, serviceId);
connection.commit(); connection.commit();
return recipientId; return recipientId;
@ -193,7 +189,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
throw new RuntimeException("Failed read recipient store", e); throw new RuntimeException("Failed read recipient store", e);
} }
} }
}
/** /**
* Should only be used for recipientIds from the database. * Should only be used for recipientIds from the database.
@ -258,7 +253,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} }
public RecipientId resolveRecipient(RecipientAddress address) { public RecipientId resolveRecipient(RecipientAddress address) {
synchronized (recipientsLock) {
final RecipientId recipientId; final RecipientId recipientId;
try (final var connection = database.getConnection()) { try (final var connection = database.getConnection()) {
connection.setAutoCommit(false); connection.setAutoCommit(false);
@ -269,7 +263,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} }
return recipientId; return recipientId;
} }
}
public RecipientId resolveRecipient(Connection connection, RecipientAddress address) throws SQLException { public RecipientId resolveRecipient(Connection connection, RecipientAddress address) throws SQLException {
return resolveRecipientLocked(connection, address); return resolveRecipientLocked(connection, address);
@ -550,10 +543,9 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
public void deleteRecipientData(RecipientId recipientId) { public void deleteRecipientData(RecipientId recipientId) {
logger.debug("Deleting recipient data for {}", recipientId); logger.debug("Deleting recipient data for {}", recipientId);
synchronized (recipientsLock) {
recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(recipientId));
try (final var connection = database.getConnection()) { try (final var connection = database.getConnection()) {
connection.setAutoCommit(false); connection.setAutoCommit(false);
recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(recipientId));
storeContact(connection, recipientId, null); storeContact(connection, recipientId, null);
storeProfile(connection, recipientId, null); storeProfile(connection, recipientId, null);
storeProfileKey(connection, recipientId, null, false); storeProfileKey(connection, recipientId, null, false);
@ -564,7 +556,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
throw new RuntimeException("Failed update recipient store", e); throw new RuntimeException("Failed update recipient store", e);
} }
} }
}
@Override @Override
public Profile getProfile(final RecipientId recipientId) { public Profile getProfile(final RecipientId recipientId) {
@ -1026,7 +1017,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
private RecipientId resolveRecipientTrusted(RecipientAddress address, boolean isSelf) { private RecipientId resolveRecipientTrusted(RecipientAddress address, boolean isSelf) {
final Pair<RecipientId, List<RecipientId>> pair; final Pair<RecipientId, List<RecipientId>> pair;
synchronized (recipientsLock) {
try (final var connection = database.getConnection()) { try (final var connection = database.getConnection()) {
connection.setAutoCommit(false); connection.setAutoCommit(false);
pair = resolveRecipientTrustedLocked(connection, address, isSelf); pair = resolveRecipientTrustedLocked(connection, address, isSelf);
@ -1034,7 +1024,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} catch (SQLException e) { } catch (SQLException e) {
throw new RuntimeException("Failed update recipient store", e); throw new RuntimeException("Failed update recipient store", e);
} }
}
if (!pair.second().isEmpty()) { if (!pair.second().isEmpty()) {
logger.debug("Resolved address {}, merging {} other recipients", address, pair.second().size()); logger.debug("Resolved address {}, merging {} other recipients", address, pair.second().size());
@ -1073,11 +1062,9 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
for (final var toBeMergedRecipientId : toBeMergedRecipientIds) { for (final var toBeMergedRecipientId : toBeMergedRecipientIds) {
recipientMergeHandler.mergeRecipients(connection, recipientId, toBeMergedRecipientId); recipientMergeHandler.mergeRecipients(connection, recipientId, toBeMergedRecipientId);
deleteRecipient(connection, toBeMergedRecipientId); deleteRecipient(connection, toBeMergedRecipientId);
synchronized (recipientsLock) {
recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(toBeMergedRecipientId)); recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(toBeMergedRecipientId));
} }
} }
}
private RecipientId resolveRecipientLocked( private RecipientId resolveRecipientLocked(
Connection connection, RecipientAddress address Connection connection, RecipientAddress address
@ -1164,7 +1151,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} }
private void removeRecipientAddress(Connection connection, RecipientId recipientId) throws SQLException { private void removeRecipientAddress(Connection connection, RecipientId recipientId) throws SQLException {
synchronized (recipientsLock) {
recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(recipientId)); recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(recipientId));
final var sql = ( final var sql = (
""" """
@ -1178,12 +1164,10 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
statement.executeUpdate(); statement.executeUpdate();
} }
} }
}
private void updateRecipientAddress( private void updateRecipientAddress(
Connection connection, RecipientId recipientId, final RecipientAddress address Connection connection, RecipientId recipientId, final RecipientAddress address
) throws SQLException { ) throws SQLException {
synchronized (recipientsLock) {
recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(recipientId)); recipientAddressCache.entrySet().removeIf(e -> e.getValue().id().equals(recipientId));
final var sql = ( final var sql = (
""" """
@ -1202,7 +1186,6 @@ public class RecipientStore implements RecipientIdCreator, RecipientResolver, Re
} }
rotateStorageId(connection, recipientId); rotateStorageId(connection, recipientId);
} }
}
private void deleteRecipient(final Connection connection, final RecipientId recipientId) throws SQLException { private void deleteRecipient(final Connection connection, final RecipientId recipientId) throws SQLException {
final var sql = ( final var sql = (