remove neccessity for FileLock on set_password

chatmaild/src/chatmaild/doveauth.py

@@ -3,7 +3,6 @@ import json
 import logging
 import os
 import sys
-from threading import RLock
 
 from .config import Config, read_config
 from .dictproxy import DictProxy
@@ -86,10 +85,6 @@ class AuthDictProxy(DictProxy):
     def __init__(self, config):
         super().__init__()
         self.config = config
-        # We serialize all password-writes in the single doveauth process
-        # so that threads can not mangle the password when writing.
-        # Setting a password is a quite rare event anyway.
-        self._password_write_lock = RLock()
 
     def handle_lookup(self, parts):
         # Dovecot <2.3.17 has only one part,
@@ -145,8 +140,7 @@ class AuthDictProxy(DictProxy):
         if not is_allowed_to_create(self.config, addr, cleartext_password):
             return
 
-        with self._password_write_lock:
-            user.set_password(encrypt_password(cleartext_password))
+        user.set_password(encrypt_password(cleartext_password))
         print(f"Created address: {user}", file=sys.stderr)
         return user.get_userdb_dict()
 

chatmaild/src/chatmaild/filedict.py

@@ -2,6 +2,7 @@ import json
 import logging
 import os
 from contextlib import contextmanager
+from random import randint
 
 import filelock
 
@@ -34,3 +35,10 @@ class FileDict:
         except Exception:
             logging.warning(f"corrupt serialization state at: {self.path!r}")
             return {}
+
+
+def write_bytes_atomic(path, content):
+    rint = randint(0, 10000000)
+    tmp = path.with_name(path.name + f".tmp-{rint}")
+    tmp.write_bytes(content)
+    os.rename(tmp, path)

chatmaild/src/chatmaild/tests/test_filedict.py

@@ -1,4 +1,6 @@
-from chatmaild.filedict import FileDict
+import threading
+
+from chatmaild.filedict import FileDict, write_bytes_atomic
 
 
 def test_basic(tmp_path):
@@ -17,3 +19,21 @@ def test_bad_marshal_file(tmp_path, caplog):
     fdict1.path.write_bytes(b"l12k3l12k3l")
     assert fdict1.read() == {}
     assert "corrupt" in caplog.records[0].msg
+
+
+def test_write_bytes_atomic_concurrent(tmp_path):
+    p = tmp_path.joinpath("somefile.ext")
+    write_bytes_atomic(p, b"hello")
+
+    threads = []
+    for i in range(30):
+        content = f"hello{i}".encode("ascii")
+        t = threading.Thread(target=lambda: write_bytes_atomic(p, content))
+        t.start()
+        threads.append(t)
+
+    for t in threads:
+        t.join()
+
+    assert p.read_text().strip() != "hello"
+    assert len(list(p.parent.iterdir())) == 1

chatmaild/src/chatmaild/user.py

@@ -1,6 +1,8 @@
 import logging
 import os
 
+from chatmaild.filedict import write_bytes_atomic
+
 
 def get_daytimestamp(timestamp) -> int:
     return int(timestamp) // 86400 * 86400
@@ -37,15 +39,14 @@ class User:
     def set_password(self, enc_password):
         """Set the specified password for this user.
 
-        NOTE that this method is not multi-thread/process safe.
-        The caller has to ensure only a single thread writes to the same
-        user's password file.
+        This method can be called concurrently
+        but there is no guarantee which of the password-set calls will win.
         """
         self.maildir.mkdir(exist_ok=True, parents=True)
         password = enc_password.encode("ascii")
 
         try:
-            self.password_path.write_bytes(password)
+            write_bytes_atomic(self.password_path, password)
         except PermissionError:
             if not self.addr.startswith("echo@"):
                 logging.error(f"could not write password for: {self.addr}")