From d520023fc76a522650b7561f2a4fc7a95fb5a04d Mon Sep 17 00:00:00 2001 From: AsamK Date: Wed, 13 May 2020 23:33:40 +0200 Subject: [PATCH] Refactor Manager and SignalAccount to implement Closeable Should make sure that file lock and web socket connections are closed reliably. --- src/main/java/org/asamk/signal/Main.java | 177 ++++++++++-------- .../org/asamk/signal/manager/Manager.java | 116 ++++++------ .../signal/manager/ProvisioningManager.java | 21 ++- .../asamk/signal/storage/SignalAccount.java | 66 ++++--- 4 files changed, 220 insertions(+), 160 deletions(-) diff --git a/src/main/java/org/asamk/signal/Main.java b/src/main/java/org/asamk/signal/Main.java index 0958b9a8..16d9e0dc 100644 --- a/src/main/java/org/asamk/signal/Main.java +++ b/src/main/java/org/asamk/signal/Main.java @@ -44,6 +44,7 @@ import org.whispersystems.signalservice.api.push.exceptions.AuthorizationFailedE import org.whispersystems.signalservice.api.util.PhoneNumberFormatter; import java.io.File; +import java.io.IOException; import java.security.Security; import java.util.Map; @@ -71,92 +72,120 @@ public class Main { private static int handleCommands(Namespace ns) { final String username = ns.getString("username"); - Manager m = null; - ProvisioningManager pm = null; - Signal ts; - DBusConnection dBusConn = null; - try { - if (ns.getBoolean("dbus") || ns.getBoolean("dbus_system")) { - try { - DBusConnection.DBusBusType busType; - if (ns.getBoolean("dbus_system")) { - busType = DBusConnection.DBusBusType.SYSTEM; - } else { - busType = DBusConnection.DBusBusType.SESSION; - } - dBusConn = DBusConnection.getConnection(busType); - ts = dBusConn.getRemoteObject( + + if (ns.getBoolean("dbus") || ns.getBoolean("dbus_system")) { + try { + DBusConnection.DBusBusType busType; + if (ns.getBoolean("dbus_system")) { + busType = DBusConnection.DBusBusType.SYSTEM; + } else { + busType = DBusConnection.DBusBusType.SESSION; + } + try (DBusConnection dBusConn = DBusConnection.getConnection(busType)) { + Signal ts = dBusConn.getRemoteObject( DbusConfig.SIGNAL_BUSNAME, DbusConfig.SIGNAL_OBJECTPATH, Signal.class); - } catch (UnsatisfiedLinkError e) { - System.err.println("Missing native library dependency for dbus service: " + e.getMessage()); - return 1; - } catch (DBusException e) { - e.printStackTrace(); - if (dBusConn != null) { - dBusConn.disconnect(); - } - return 3; - } - } else { - String dataPath = ns.getString("config"); - if (isEmpty(dataPath)) { - dataPath = getDefaultDataPath(); + + return handleCommands(ns, ts, dBusConn); } + } catch (UnsatisfiedLinkError e) { + System.err.println("Missing native library dependency for dbus service: " + e.getMessage()); + return 1; + } catch (DBusException | IOException e) { + e.printStackTrace(); + return 3; + } + } else { + String dataPath = ns.getString("config"); + if (isEmpty(dataPath)) { + dataPath = getDefaultDataPath(); + } - if (username == null) { - pm = new ProvisioningManager(dataPath, ServiceConfig.createDefaultServiceConfiguration(BaseConfig.USER_AGENT), BaseConfig.USER_AGENT); - ts = null; - } else { - try { - m = Manager.init(username, dataPath, ServiceConfig.createDefaultServiceConfiguration(BaseConfig.USER_AGENT), BaseConfig.USER_AGENT); - } catch (AuthorizationFailedException e) { - if (!"register".equals(ns.getString("command"))) { - // Register command should still be possible, if current authorization fails - System.err.println("Authorization failed, was the number registered elsewhere?"); - return 2; - } - } catch (Throwable e) { - System.err.println("Error loading state file: " + e.getMessage()); + if (username == null) { + ProvisioningManager pm = new ProvisioningManager(dataPath, ServiceConfig.createDefaultServiceConfiguration(BaseConfig.USER_AGENT), BaseConfig.USER_AGENT); + return handleCommands(ns, pm); + } + + Manager manager; + try { + manager = Manager.init(username, dataPath, ServiceConfig.createDefaultServiceConfiguration(BaseConfig.USER_AGENT), BaseConfig.USER_AGENT); + } catch (Throwable e) { + System.err.println("Error loading state file: " + e.getMessage()); + return 2; + } + + try (Manager m = manager) { + try { + m.checkAccountState(); + } catch (AuthorizationFailedException e) { + if (!"register".equals(ns.getString("command"))) { + // Register command should still be possible, if current authorization fails + System.err.println("Authorization failed, was the number registered elsewhere?"); return 2; } - ts = m; + } catch (IOException e) { + System.err.println("Error while checking account: " + e.getMessage()); + return 2; } + + return handleCommands(ns, m); + } catch (IOException e) { + e.printStackTrace(); + return 3; + } + } + } + + private static int handleCommands(Namespace ns, Signal ts, DBusConnection dBusConn) { + String commandKey = ns.getString("command"); + final Map commands = Commands.getCommands(); + if (commands.containsKey(commandKey)) { + Command command = commands.get(commandKey); + + if (command instanceof ExtendedDbusCommand) { + return ((ExtendedDbusCommand) command).handleCommand(ns, ts, dBusConn); + } else if (command instanceof DbusCommand) { + return ((DbusCommand) command).handleCommand(ns, ts); + } else { + System.err.println(commandKey + " is not yet implemented via dbus"); + return 1; } + } + return 0; + } - String commandKey = ns.getString("command"); - final Map commands = Commands.getCommands(); - if (commands.containsKey(commandKey)) { - Command command = commands.get(commandKey); - - if (dBusConn != null) { - if (command instanceof ExtendedDbusCommand) { - return ((ExtendedDbusCommand) command).handleCommand(ns, ts, dBusConn); - } else if (command instanceof DbusCommand) { - return ((DbusCommand) command).handleCommand(ns, ts); - } else { - System.err.println(commandKey + " is not yet implemented via dbus"); - return 1; - } - } else { - if (command instanceof LocalCommand) { - return ((LocalCommand) command).handleCommand(ns, m); - } else if (command instanceof ProvisioningCommand) { - return ((ProvisioningCommand) command).handleCommand(ns, pm); - } else if (command instanceof DbusCommand) { - return ((DbusCommand) command).handleCommand(ns, ts); - } else { - System.err.println(commandKey + " is only works via dbus"); - return 1; - } - } + private static int handleCommands(Namespace ns, ProvisioningManager pm) { + String commandKey = ns.getString("command"); + final Map commands = Commands.getCommands(); + if (commands.containsKey(commandKey)) { + Command command = commands.get(commandKey); + + if (command instanceof ProvisioningCommand) { + return ((ProvisioningCommand) command).handleCommand(ns, pm); + } else { + System.err.println(commandKey + " only works with a username"); + return 1; } - return 0; - } finally { - if (dBusConn != null) { - dBusConn.disconnect(); + } + return 0; + } + + private static int handleCommands(Namespace ns, Manager m) { + String commandKey = ns.getString("command"); + final Map commands = Commands.getCommands(); + if (commands.containsKey(commandKey)) { + Command command = commands.get(commandKey); + + if (command instanceof LocalCommand) { + return ((LocalCommand) command).handleCommand(ns, m); + } else if (command instanceof DbusCommand) { + return ((DbusCommand) command).handleCommand(ns, m); + } else if (command instanceof ExtendedDbusCommand) { + System.err.println(commandKey + " only works via dbus"); } + return 1; } + return 0; } /** diff --git a/src/main/java/org/asamk/signal/manager/Manager.java b/src/main/java/org/asamk/signal/manager/Manager.java index 1340cd0a..7d15b8a1 100644 --- a/src/main/java/org/asamk/signal/manager/Manager.java +++ b/src/main/java/org/asamk/signal/manager/Manager.java @@ -115,6 +115,7 @@ import org.whispersystems.signalservice.internal.push.VerifyAccountResponse; import org.whispersystems.signalservice.internal.util.Hex; import org.whispersystems.util.Base64; +import java.io.Closeable; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -146,7 +147,7 @@ import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; -public class Manager implements Signal { +public class Manager implements Signal, Closeable { private final SleepTimer timer = new UptimeSleepTimer(); private final SignalServiceConfiguration serviceConfiguration; @@ -225,7 +226,6 @@ public class Manager implements Signal { Manager m = new Manager(account, pathConfig, serviceConfiguration, userAgent); m.migrateLegacyConfigs(); - m.checkAccountState(); return m; } @@ -256,7 +256,7 @@ public class Manager implements Signal { } } - private void checkAccountState() throws IOException { + public void checkAccountState() throws IOException { if (account.isRegistered()) { if (accountManager.getPreKeysCount() < ServiceConfig.PREKEY_MINIMUM_COUNT) { refreshPreKeys(); @@ -1422,63 +1422,56 @@ public class Manager implements Signal { retryFailedReceivedMessages(handler, ignoreAttachments); final SignalServiceMessageReceiver messageReceiver = getMessageReceiver(); - try { - if (messagePipe == null) { - messagePipe = messageReceiver.createMessagePipe(); - } + if (messagePipe == null) { + messagePipe = messageReceiver.createMessagePipe(); + } - while (true) { - SignalServiceEnvelope envelope; - SignalServiceContent content = null; - Exception exception = null; - final long now = new Date().getTime(); - try { - envelope = messagePipe.read(timeout, unit, envelope1 -> { - // store message on disk, before acknowledging receipt to the server - try { - String source = envelope1.getSourceE164().isPresent() ? envelope1.getSourceE164().get() : ""; - File cacheFile = getMessageCacheFile(source, now, envelope1.getTimestamp()); - Utils.storeEnvelope(envelope1, cacheFile); - } catch (IOException e) { - System.err.println("Failed to store encrypted message in disk cache, ignoring: " + e.getMessage()); - } - }); - } catch (TimeoutException e) { - if (returnOnTimeout) - return; - continue; - } catch (InvalidVersionException e) { - System.err.println("Ignoring error: " + e.getMessage()); - continue; - } - if (!envelope.isReceipt()) { - try { - content = decryptMessage(envelope); - } catch (Exception e) { - exception = e; - } - handleMessage(envelope, content, ignoreAttachments); - } - account.save(); - if (!isMessageBlocked(envelope, content)) { - handler.handleMessage(envelope, content, exception); - } - if (!(exception instanceof org.whispersystems.libsignal.UntrustedIdentityException)) { - File cacheFile = null; + while (true) { + SignalServiceEnvelope envelope; + SignalServiceContent content = null; + Exception exception = null; + final long now = new Date().getTime(); + try { + envelope = messagePipe.read(timeout, unit, envelope1 -> { + // store message on disk, before acknowledging receipt to the server try { - cacheFile = getMessageCacheFile(envelope.getSourceE164().get(), now, envelope.getTimestamp()); - Files.delete(cacheFile.toPath()); - // Try to delete directory if empty - new File(getMessageCachePath()).delete(); + String source = envelope1.getSourceE164().isPresent() ? envelope1.getSourceE164().get() : ""; + File cacheFile = getMessageCacheFile(source, now, envelope1.getTimestamp()); + Utils.storeEnvelope(envelope1, cacheFile); } catch (IOException e) { - System.err.println("Failed to delete cached message file “" + cacheFile + "”: " + e.getMessage()); + System.err.println("Failed to store encrypted message in disk cache, ignoring: " + e.getMessage()); } + }); + } catch (TimeoutException e) { + if (returnOnTimeout) + return; + continue; + } catch (InvalidVersionException e) { + System.err.println("Ignoring error: " + e.getMessage()); + continue; + } + if (!envelope.isReceipt()) { + try { + content = decryptMessage(envelope); + } catch (Exception e) { + exception = e; } + handleMessage(envelope, content, ignoreAttachments); } - } finally { - if (messagePipe != null) { - messagePipe.shutdown(); - messagePipe = null; + account.save(); + if (!isMessageBlocked(envelope, content)) { + handler.handleMessage(envelope, content, exception); + } + if (!(exception instanceof org.whispersystems.libsignal.UntrustedIdentityException)) { + File cacheFile = null; + try { + cacheFile = getMessageCacheFile(envelope.getSourceE164().get(), now, envelope.getTimestamp()); + Files.delete(cacheFile.toPath()); + // Try to delete directory if empty + new File(getMessageCachePath()).delete(); + } catch (IOException e) { + System.err.println("Failed to delete cached message file “" + cacheFile + "”: " + e.getMessage()); + } } } } @@ -2026,6 +2019,21 @@ public class Manager implements Signal { return account.getRecipientStore().resolveServiceAddress(address); } + @Override + public void close() throws IOException { + if (messagePipe != null) { + messagePipe.shutdown(); + messagePipe = null; + } + + if (unidentifiedMessagePipe != null) { + unidentifiedMessagePipe.shutdown(); + unidentifiedMessagePipe = null; + } + + account.close(); + } + public interface ReceiveMessageHandler { void handleMessage(SignalServiceEnvelope envelope, SignalServiceContent decryptedContent, Throwable e); diff --git a/src/main/java/org/asamk/signal/manager/ProvisioningManager.java b/src/main/java/org/asamk/signal/manager/ProvisioningManager.java index 61d3315f..e7693f21 100644 --- a/src/main/java/org/asamk/signal/manager/ProvisioningManager.java +++ b/src/main/java/org/asamk/signal/manager/ProvisioningManager.java @@ -83,19 +83,22 @@ public class ProvisioningManager { throw new IOException("Received invalid profileKey", e); } } - SignalAccount account = SignalAccount.createLinkedAccount(pathConfig.getDataPath(), username, ret.getUuid(), password, ret.getDeviceId(), ret.getIdentity(), registrationId, signalingKey, profileKey); - account.save(); - Manager m = new Manager(account, pathConfig, serviceConfiguration, userAgent); + try (SignalAccount account = SignalAccount.createLinkedAccount(pathConfig.getDataPath(), username, ret.getUuid(), password, ret.getDeviceId(), ret.getIdentity(), registrationId, signalingKey, profileKey)) { + account.save(); - m.refreshPreKeys(); + try (Manager m = new Manager(account, pathConfig, serviceConfiguration, userAgent)) { - m.requestSyncGroups(); - m.requestSyncContacts(); - m.requestSyncBlocked(); - m.requestSyncConfiguration(); + m.refreshPreKeys(); - m.saveAccount(); + m.requestSyncGroups(); + m.requestSyncContacts(); + m.requestSyncBlocked(); + m.requestSyncConfiguration(); + + m.saveAccount(); + } + } return username; } diff --git a/src/main/java/org/asamk/signal/storage/SignalAccount.java b/src/main/java/org/asamk/signal/storage/SignalAccount.java index 16ad2122..94935441 100644 --- a/src/main/java/org/asamk/signal/storage/SignalAccount.java +++ b/src/main/java/org/asamk/signal/storage/SignalAccount.java @@ -29,9 +29,11 @@ import org.whispersystems.libsignal.IdentityKeyPair; import org.whispersystems.libsignal.state.PreKeyRecord; import org.whispersystems.libsignal.state.SignedPreKeyRecord; import org.whispersystems.libsignal.util.Medium; +import org.whispersystems.libsignal.util.Pair; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.util.Base64; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; @@ -42,11 +44,11 @@ import java.util.Collection; import java.util.UUID; import java.util.stream.Collectors; -public class SignalAccount { +public class SignalAccount implements Closeable { private final ObjectMapper jsonProcessor = new ObjectMapper(); - private FileChannel fileChannel; - private FileLock lock; + private final FileChannel fileChannel; + private final FileLock lock; private String username; private UUID uuid; private int deviceId = SignalServiceAddress.DEFAULT_DEVICE_ID; @@ -65,7 +67,9 @@ public class SignalAccount { private JsonContactsStore contactStore; private RecipientStore recipientStore; - private SignalAccount() { + private SignalAccount(final FileChannel fileChannel, final FileLock lock) { + this.fileChannel = fileChannel; + this.lock = lock; jsonProcessor.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE); // disable autodetect jsonProcessor.enable(SerializationFeature.INDENT_OUTPUT); // for pretty print, you can disable it. jsonProcessor.enable(SerializationFeature.WRITE_NULL_MAP_VALUES); @@ -75,18 +79,28 @@ public class SignalAccount { } public static SignalAccount load(String dataPath, String username) throws IOException { - SignalAccount account = new SignalAccount(); - IOUtils.createPrivateDirectories(dataPath); - account.openFileChannel(getFileName(dataPath, username)); - account.load(); - return account; + final String fileName = getFileName(dataPath, username); + final Pair pair = openFileChannel(fileName); + try { + SignalAccount account = new SignalAccount(pair.first(), pair.second()); + account.load(); + return account; + } catch (Throwable e) { + pair.second().close(); + pair.first().close(); + throw e; + } } public static SignalAccount create(String dataPath, String username, IdentityKeyPair identityKey, int registrationId, ProfileKey profileKey) throws IOException { IOUtils.createPrivateDirectories(dataPath); + String fileName = getFileName(dataPath, username); + if (!new File(fileName).exists()) { + IOUtils.createPrivateFile(fileName); + } - SignalAccount account = new SignalAccount(); - account.openFileChannel(getFileName(dataPath, username)); + final Pair pair = openFileChannel(fileName); + SignalAccount account = new SignalAccount(pair.first(), pair.second()); account.username = username; account.profileKey = profileKey; @@ -101,9 +115,13 @@ public class SignalAccount { public static SignalAccount createLinkedAccount(String dataPath, String username, UUID uuid, String password, int deviceId, IdentityKeyPair identityKey, int registrationId, String signalingKey, ProfileKey profileKey) throws IOException { IOUtils.createPrivateDirectories(dataPath); + String fileName = getFileName(dataPath, username); + if (!new File(fileName).exists()) { + IOUtils.createPrivateFile(fileName); + } - SignalAccount account = new SignalAccount(); - account.openFileChannel(getFileName(dataPath, username)); + final Pair pair = openFileChannel(fileName); + SignalAccount account = new SignalAccount(pair.first(), pair.second()); account.username = username; account.uuid = uuid; @@ -285,21 +303,15 @@ public class SignalAccount { } } - private void openFileChannel(String fileName) throws IOException { - if (fileChannel != null) { - return; - } - - if (!new File(fileName).exists()) { - IOUtils.createPrivateFile(fileName); - } - fileChannel = new RandomAccessFile(new File(fileName), "rw").getChannel(); - lock = fileChannel.tryLock(); + private static Pair openFileChannel(String fileName) throws IOException { + FileChannel fileChannel = new RandomAccessFile(new File(fileName), "rw").getChannel(); + FileLock lock = fileChannel.tryLock(); if (lock == null) { System.err.println("Config file is in use by another instance, waiting…"); lock = fileChannel.lock(); System.err.println("Config file lock acquired."); } + return new Pair<>(fileChannel, lock); } public void setResolver(final SignalServiceAddressResolver resolver) { @@ -413,4 +425,12 @@ public class SignalAccount { public void setMultiDevice(final boolean multiDevice) { isMultiDevice = multiDevice; } + + @Override + public void close() throws IOException { + synchronized (fileChannel) { + lock.close(); + fileChannel.close(); + } + } } -- 2.50.1