email: Drop use of mutex for postfix configuration operations

This is not critically needed in FreedomBox. Implementation is complex. This may
be recreated at framework level to benefit all applications.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/email/lock.py

@@ -1,106 +0,0 @@
-# SPDX-License-Identifier: AGPL-3.0-or-later
-import contextlib
-import fcntl
-import logging
-import os
-import pwd
-import re
-import subprocess
-import threading
-
-from . import interproc
-
-lock_name_pattern = re.compile('^[0-9a-zA-Z_-]+$')
-logger = logging.getLogger(__name__)
-
-
-class RaceCondition(AssertionError):
-    pass
-
-
-class Mutex:
-    """File and pthread lock based resource mutex"""
-
-    def __init__(self, lock_name):
-        if not lock_name_pattern.match(lock_name):
-            raise ValueError('Bad lock name')
-        self._lock_path = '/var/lock/plinth-%s.lock' % lock_name
-        self._thread_mutex = threading.Lock()
-
-    @property
-    def lock_path(self):
-        return self._lock_path
-
-    @contextlib.contextmanager
-    def lock_threads_only(self):
-        """Acquire the thread lock but not the file lock"""
-        if not self._thread_mutex.acquire(timeout=5):
-            raise RuntimeError('Could not acquire thread lock')
-        try:
-            yield
-        finally:
-            self._thread_mutex.release()
-
-    @contextlib.contextmanager
-    def lock_all(self):
-        """Acquire both the thread lock and the file lock"""
-        with self.lock_threads_only():
-            # Set up
-            fd = self._open_lock_file()
-            fcntl.lockf(fd, fcntl.LOCK_EX)
-            # Enter context
-            try:
-                yield
-            finally:
-                # Clean up
-                fcntl.lockf(fd, fcntl.LOCK_UN)
-                fd.close()
-
-    def _open_lock_file(self):
-        """Attempt to open lock file for R&W. Raises OSError on failure"""
-        og_ruid, og_euid, og_suid = os.getresuid()
-        if og_euid == 0 and threading.active_count() > 1:
-            raise RaceCondition('setuid in a multi-threaded process')
-        if not os.path.exists(self.lock_path):
-            self._create_lock_file_as_plinth(og_euid)
-
-        fd = None
-        try:
-            if og_euid == 0:
-                # Temporarily run the current process as plinth
-                plinth_uid = pwd.getpwnam('plinth').pw_uid
-                self._checked_setresuid(og_ruid, plinth_uid, 0)
-            fd = open(self.lock_path, 'w+b')
-        finally:
-            # Restore resuid
-            if og_euid == 0:
-                self._checked_setresuid(og_ruid, 0, 0)
-                if og_suid != 0:
-                    self._checked_setresuid(og_ruid, 0, og_suid)
-
-        return fd
-
-    def _create_lock_file_as_plinth(self, your_euid):
-        # Don't change the current processes umask
-        # Do create a new process
-        args = []
-        if your_euid == 0:
-            args.extend(['sudo', '-n', '-u', 'plinth'])
-        args.extend(['/bin/sh', '-c'])
-        args.append('umask 177 && > ' + self.lock_path)
-
-        completed = subprocess.run(args, capture_output=True, check=False)
-        if completed.returncode != 0:
-            interproc.log_subprocess(completed)
-            raise OSError('Could not create ' + self.lock_path)
-
-    def _checked_setresuid(self, ruid, euid, suid):
-        os.setresuid(ruid, euid, suid)
-        if os.getresuid() != (ruid, euid, suid):
-            try:
-                raise SystemExit('PANIC: setresuid failed')
-            except SystemExit as e:
-                # Print stack trace
-                logger.exception(e)
-                # Force exit
-                exit(1)

plinth/modules/email/postconf.py

@@ -8,10 +8,8 @@ from dataclasses import dataclass
 from typing import ClassVar
 
 from . import interproc
-from .lock import Mutex
 
 logger = logging.getLogger(__name__)
-mutex = Mutex('email-postconf')
 
 
 @dataclass
@@ -28,8 +26,10 @@ class ServiceFlags:
     crash_handler: ClassVar[str] = '/dev/null/plinth-crash'
 
     def _get_flags_ordered(self):
-        return [self.service, self.type, self.private, self.unpriv,
+        return [
-                self.chroot, self.wakeup, self.maxproc, self.command_args]
+            self.service, self.type, self.private, self.unpriv, self.chroot,
+            self.wakeup, self.maxproc, self.command_args
+        ]
 
     def serialize(self) -> str:
         ordered = self._get_flags_ordered()
@@ -57,8 +57,8 @@ def get_many(key_list):
     Return a key-value map"""
     for key in key_list:
         validate_key(key)
-    with mutex.lock_all():
+
-        return get_many_unsafe(key_list)
+    return get_many_unsafe(key_list)
 
 
 def get_many_unsafe(key_iterator, flag=''):
@@ -90,8 +90,7 @@ def set_many(kv_map):
         validate_key(key)
         validate_value(value)
 
-    with mutex.lock_all():
+    set_many_unsafe(kv_map)
-        set_many_unsafe(kv_map)
 
 
 def set_many_unsafe(kv_map, flag=''):
@@ -123,10 +122,9 @@ def set_master_cf_options(service_flags, options={}):
     # /sbin/postconf -M "service/type=<temp flag string>"
     # /sbin/postconf -P "service/type/k=v" ...
     # Delete placeholder string /dev/null/plinth-crash
-    with mutex.lock_all():
+    set_unsafe(service_key, service_flags.serialize_temp(), '-M')
-        set_unsafe(service_key, service_flags.serialize_temp(), '-M')
+    set_many_unsafe(long_opts, '-P')
-        set_many_unsafe(long_opts, '-P')
+    _master_remove_crash_handler(service_flags)
-        _master_remove_crash_handler(service_flags)
 
 
 def get_unsafe(key):