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

@@ -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,94 +72,122 @@ public class Main {
 
     private static int handleCommands(Namespace ns) {
         final String username = ns.getString("username");
-        Manager m = null;
+
-        ProvisioningManager pm = null;
+        if (ns.getBoolean("dbus") || ns.getBoolean("dbus_system")) {
-        Signal ts;
+            try {
-        DBusConnection dBusConn = null;
+                DBusConnection.DBusBusType busType;
-        try {
+                if (ns.getBoolean("dbus_system")) {
-            if (ns.getBoolean("dbus") || ns.getBoolean("dbus_system")) {
+                    busType = DBusConnection.DBusBusType.SYSTEM;
-                try {
+                } else {
-                    DBusConnection.DBusBusType busType;
+                    busType = DBusConnection.DBusBusType.SESSION;
-                    if (ns.getBoolean("dbus_system")) {
+                }
-                        busType = DBusConnection.DBusBusType.SYSTEM;
+                try (DBusConnection dBusConn = DBusConnection.getConnection(busType)) {
-                    } else {
+                    Signal ts = dBusConn.getRemoteObject(
-                        busType = DBusConnection.DBusBusType.SESSION;
-                    }
-                    dBusConn = DBusConnection.getConnection(busType);
-                    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();
-                }
 
-                if (username == null) {
+                    return handleCommands(ns, ts, dBusConn);
-                    pm = new ProvisioningManager(dataPath, ServiceConfig.createDefaultServiceConfiguration(BaseConfig.USER_AGENT), BaseConfig.USER_AGENT);
+                }
-                    ts = null;
+            } catch (UnsatisfiedLinkError e) {
-                } else {
+                System.err.println("Missing native library dependency for dbus service: " + e.getMessage());
-                    try {
+                return 1;
-                        m = Manager.init(username, dataPath, ServiceConfig.createDefaultServiceConfiguration(BaseConfig.USER_AGENT), BaseConfig.USER_AGENT);
+            } catch (DBusException | IOException e) {
-                    } catch (AuthorizationFailedException e) {
+                e.printStackTrace();
-                        if (!"register".equals(ns.getString("command"))) {
+                return 3;
-                            // Register command should still be possible, if current authorization fails
+            }
-                            System.err.println("Authorization failed, was the number registered elsewhere?");
+        } else {
-                            return 2;
+            String dataPath = ns.getString("config");
-                        }
+            if (isEmpty(dataPath)) {
-                    } catch (Throwable e) {
+                dataPath = getDefaultDataPath();
-                        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;
                 }
-            }
 
-            String commandKey = ns.getString("command");
+                return handleCommands(ns, m);
-            final Map<String, Command> commands = Commands.getCommands();
+            } catch (IOException e) {
-            if (commands.containsKey(commandKey)) {
+                e.printStackTrace();
-                Command command = commands.get(commandKey);
+                return 3;
-
-                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;
-                    }
-                }
-            }
-            return 0;
-        } finally {
-            if (dBusConn != null) {
-                dBusConn.disconnect();
             }
         }
     }
 
+    private static int handleCommands(Namespace ns, Signal ts, DBusConnection dBusConn) {
+        String commandKey = ns.getString("command");
+        final Map<String, Command> 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;
+    }
+
+    private static int handleCommands(Namespace ns, ProvisioningManager pm) {
+        String commandKey = ns.getString("command");
+        final Map<String, Command> 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;
+    }
+
+    private static int handleCommands(Namespace ns, Manager m) {
+        String commandKey = ns.getString("command");
+        final Map<String, Command> 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;
+    }
+
     /**
      * Uses $XDG_DATA_HOME/signal-cli if it exists, or if none of the legacy directories exist:
      * - $HOME/.config/signal

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) {
-            if (messagePipe == null) {
+            messagePipe = messageReceiver.createMessagePipe();
-                messagePipe = messageReceiver.createMessagePipe();
+        }
-            }
 
-            while (true) {
+        while (true) {
-                SignalServiceEnvelope envelope;
+            SignalServiceEnvelope envelope;
-                SignalServiceContent content = null;
+            SignalServiceContent content = null;
-                Exception exception = null;
+            Exception exception = null;
-                final long now = new Date().getTime();
+            final long now = new Date().getTime();
-                try {
+            try {
-                    envelope = messagePipe.read(timeout, unit, envelope1 -> {
+                envelope = messagePipe.read(timeout, unit, envelope1 -> {
-                        // store message on disk, before acknowledging receipt to the server
+                    // 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);
+                        String source = envelope1.getSourceE164().isPresent() ? envelope1.getSourceE164().get() : "";
-                    } catch (Exception e) {
+                        File cacheFile = getMessageCacheFile(source, now, envelope1.getTimestamp());
-                        exception = e;
+                        Utils.storeEnvelope(envelope1, cacheFile);
-                    }
-                    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;
-                    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());
+                        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;
             }
-        } finally {
+            if (!envelope.isReceipt()) {
-            if (messagePipe != null) {
+                try {
-                messagePipe.shutdown();
+                    content = decryptMessage(envelope);
-                messagePipe = null;
+                } 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;
+                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);

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.refreshPreKeys();
-        m.requestSyncContacts();
-        m.requestSyncBlocked();
-        m.requestSyncConfiguration();
 
-        m.saveAccount();
+                m.requestSyncGroups();
+                m.requestSyncContacts();
+                m.requestSyncBlocked();
+                m.requestSyncConfiguration();
+
+                m.saveAccount();
+            }
+        }
 
         return username;
     }

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 final FileChannel fileChannel;
-    private FileLock lock;
+    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();
+        final String fileName = getFileName(dataPath, username);
-        IOUtils.createPrivateDirectories(dataPath);
+        final Pair<FileChannel, FileLock> pair = openFileChannel(fileName);
-        account.openFileChannel(getFileName(dataPath, username));
+        try {
-        account.load();
+            SignalAccount account = new SignalAccount(pair.first(), pair.second());
-        return account;
+            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();
+        final Pair<FileChannel, FileLock> pair = openFileChannel(fileName);
-        account.openFileChannel(getFileName(dataPath, username));
+        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();
+        final Pair<FileChannel, FileLock> pair = openFileChannel(fileName);
-        account.openFileChannel(getFileName(dataPath, username));
+        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 {
+    private static Pair<FileChannel, FileLock> openFileChannel(String fileName) throws IOException {
-        if (fileChannel != null) {
+        FileChannel fileChannel = new RandomAccessFile(new File(fileName), "rw").getChannel();
-            return;
+        FileLock lock = fileChannel.tryLock();
-        }
-
-        if (!new File(fileName).exists()) {
-            IOUtils.createPrivateFile(fileName);
-        }
-        fileChannel = new RandomAccessFile(new File(fileName), "rw").getChannel();
-        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();
+        }
+    }
 }