change locking semantics a bit and add guidelines for SystemPowerManagement

The change in locking semantics is that a lock-holder can now access a
device even if other processes have locks on the device.

4 files changed, 217 insertions, 61 deletions

diff --git a/doc/spec/hal-spec-interfaces.xml b/doc/spec/hal-spec-interfaces.xml
index fe0c112e..9858c717 100644
--- a/doc/spec/hal-spec-interfaces.xml
+++ b/doc/spec/hal-spec-interfaces.xml
@@ -653,6 +653,12 @@ $ dbus-send --system --print-reply --dest=org.freedesktop.Hal \
     <para>
       This interface does not emit any signals.
     </para>
+
+    <para>
+      Implementors of power management daemons should make sure that
+      their software respects the locking guidelines described in
+      <xref linkend="interfaces"/>.
+    </para>
   </sect1>
 
   <sect1 id="interface-cpufreq">
diff --git a/doc/spec/hal-spec-locking.xml b/doc/spec/hal-spec-locking.xml
index 443ac48b..df62f0ae 100644
--- a/doc/spec/hal-spec-locking.xml
+++ b/doc/spec/hal-spec-locking.xml
@@ -5,8 +5,8 @@
   <title>Locking</title>
 
   <para>
-    As HAL enables programs in a desktop session to automatically
-    enforce the policy of the users choice, unexpected things can
+    As HAL is a mechanism that enables programs in a desktop session
+    to enforce the policy of the users choice, unexpected things can
     happen. For example, if the user is in the middle of partitioning
     a disk drive, it is desirable to keep the desktop from mounting
     partitions that have not yet been prepared with a suitable file
@@ -72,37 +72,42 @@
       also be obtained exclusively if the caller so desires. Unlike
       per-device interface locking, it is not checked at locking time
       whether the locker have access to a given device; instead
-      checking is done when callers attempt to access the interface.
+      checking is done when callers attempt to access the
+      interface.
     </para>
 
     <para>
       The algorithm used for determining if a caller is locked out is
-      shown below. A caller is locked out if:
+      shown below. A caller A is locked out of an interface IFACE on a
+      device object DEVICE if, and only if,
     </para>
 
     <orderedlist>
       <listitem> 
 	<para>
-          another caller is holding a lock on the interface on the
-          device (non-withstanding that the caller to check for holds
-          the lock himself).
+          Another caller B is holding a lock on the interface IFACE on
+          DEVICE and A don't have either a global lock on IFACE or a
+          lock on IFACE on DEVICE; or
         </para>
       </listitem>
       <listitem>
         <para>
-          Another caller is holding the global lock for the interface
-          and that other caller has access to the device in question.
+          Another caller B is holding the global lock on the
+          interface IFACE and B has access to DEVICE and and A don't
+          have either a global lock on IFACE or a lock on IFACE on
+          DEVICE.
         </para>
       </listitem>
     </orderedlist>
 
     <para>
-      In other words, a client Foo can grab a global lock, but that
-      doesn't mean Foo can lock other clients out of devices that Foo
-      doesn't have access to. Specifically a caller is not locked out
-      if he has locked an interface and he is the only one holding the
-      lock. However, if two clients have a lock on a device neither of
-      them can access the device.
+      In other words, a caller A can grab a global lock, but that
+      doesn't mean A can lock other clients out of devices that A
+      doesn't have access to. Specifically a caller is never locked
+      out if he has locked an interface either globally or on the
+      device in question. However, if two clients have a lock on a
+      device, then both can access it. To ensure that everyone is
+      locked out, a caller needs to use an exclusive lock.
     </para>
 
     <para>
@@ -135,21 +140,148 @@
           block devices directly (and pokes the kernel to reload the
           partitioning table) should lock out automounters by either
           a) obtaining
-          the <literal>org.freedesktop.Hal.Device.Storage</literal> on
-          each drive being processed; or b) obtaintaing the global
+          the <literal>org.freedesktop.Hal.Device.Storage</literal>
+          lock on each drive being processed; or b) obtaintaing the
+          global
           <literal>org.freedesktop.Hal.Device.Storage</literal>
           lock. This includes programs like fdisk, gparted, parted and
           operating system installers. See also
-          <xref linkend="interface-device-volume"/>.
+          <xref linkend="interface-device-volume"/> and
+          the <literal>hal-lock</literal>(1) program and manual page.
         </para>
       </listitem>
 
       <listitem><para>
-          <emphasis>System-wide Power Management</emphasis>
+          <emphasis>Power Management</emphasis>
         </para>
         <para>
-          (this guideline is not finalized!)
+          Typically, a desktop session includes a session-wide power
+          management daemon that enforces the policy of the users
+          choice, e.g. whether the system should suspend to ram on lid
+          close, whether to hibernate the system after the user being
+          idle for 30 minutes and so on. In a multi-user setup (both
+          fast user switching and multi-seat), this can break in
+          various interesting ways unless the power management daemons
+          cooperate. Also, there may be software running at the system
+          level who will want to inhibit a desktop session power
+          management daemon from suspending / shutting down.
         </para>
+
+        <itemizedlist>
+          <listitem>
+            <para>
+              System-level software that do not wish to be interrupted
+              by the effect of someone calling into the
+              <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface MUST hold the
+              <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              lock non-exclusively on the root computer device
+              object. For example, the YUM software updater should
+              hold the lock when doing an RPM transaction.
+            </para>
+          </listitem>
+        </itemizedlist>
+
+        <para>
+          In addition, any power management session daemon instance
+        </para>
+
+        <itemizedlist>
+          <listitem>
+            <para>
+              ... MUST hold the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal> lock
+              non-exclusively on the root computer device object
+              unless it is prepared to call into this interface
+              itself. This typically means that the PM daemon instance
+              simply acquires the lock on start up and releases it
+              just before it calls into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface. In other words, the PM daemon instance needs
+              to hold the lock exactly when it doesn't want other PM
+              daemon instances to call into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal> interface.
+              This means that if the user have configured the PM
+              daemon instance to go to sleep after 30 minutes of
+              inactivity, the lock should be released then.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              ... MUST not hold the lock when the session is inactive
+              (fast user switching) UNLESS an application in the
+              session have explicitly called Inhibit() on
+              the <literal>org.freedesktop.PowerManagement</literal>
+              D-Bus session bus interface of the PM daemon.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              ... MUST check that no other process is holding the lock
+              before calling into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface. If another process is holding the lock, it
+              means that either 1) another session is not prepared to
+              call into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface; OR 2) some system-level software is holding
+              the lock. The PM daemon instance MUST respect this by
+              not calling into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface itself.
+            </para>
+          </listitem>
+
+        </itemizedlist>
+        <para>
+          However, any Power management daemon instance
+        </para>
+        <itemizedlist>
+
+          <listitem>
+            <para>
+              ... MAY prompt the user, if applicable, to ask if she
+              still wants to perform the requested action (e.g. call
+              into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface) despite the fact that another process
+              (possibly from another user) is indicating that it does
+              not want the system to e.g. suspend. Only if the user
+              agrees, the power management instance should call into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface. Typically, it's only useful to prompt the
+              user with such questions if the request to call into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface originates from user input, e.g. either a
+              hotkey, the user clicking a suspend button in the UI or
+              an application invoking the <literal>Suspend()</literal> method on the 
+              <literal>org.freedesktop.PowerManagement</literal> D-Bus
+              session interface of the PM daemon.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              ... MAY ignore that other processes are holding the lock
+              and call into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface anyway, but ONLY if if the request to call
+              into
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface originated from e.g. lid close, critically low
+              battery or other similar conditions.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              ... MAY still call <literal>SetPowerSave()</literal> on
+              the <literal>org.freedesktop.Hal.Device.SystemPowerManagement</literal>
+              interface even if other processes are holding the lock.
+            </para>
+          </listitem>
+        </itemizedlist>
       </listitem>
       
     </itemizedlist>
diff --git a/examples/interface-locking-test.py b/examples/interface-locking-test.py
index 3b98cd1f..ca60a910 100755
--- a/examples/interface-locking-test.py
+++ b/examples/interface-locking-test.py
@@ -17,9 +17,16 @@ device = dbus.Interface(bus.get_object("org.freedesktop.Hal",
                                          "/org/freedesktop/Hal/devices/volume_uuid_2232_1F11"),
                           "org.freedesktop.Hal.Device")
 
-manager.AcquireGlobalInterfaceLock("org.freedesktop.Hal.Device.Storage", True)
-#device.AcquireInterfaceLock("org.freedesktop.Hal.Device.Volume", True)
+device_volume = dbus.Interface(bus.get_object("org.freedesktop.Hal",
+                                              "/org/freedesktop/Hal/devices/volume_uuid_2232_1F11"),
+                               "org.freedesktop.Hal.Device.Volume")
+
+#manager.AcquireGlobalInterfaceLock("org.freedesktop.Hal.Device.Storage", True)
+#time.sleep(2)
+#manager.ReleaseGlobalInterfaceLock("org.freedesktop.Hal.Device.Storage")
+
+device.AcquireInterfaceLock("org.freedesktop.Hal.Device.Volume", False)
+device_volume.Mount("", "", [])
 time.sleep(2)
-manager.ReleaseGlobalInterfaceLock("org.freedesktop.Hal.Device.Storage")
-#device.ReleaseInterfaceLock("org.freedesktop.Hal.Device.Volume")
+device.ReleaseInterfaceLock("org.freedesktop.Hal.Device.Volume")
 
diff --git a/hald/access-check.c b/hald/access-check.c
index d96ddaaa..6f0542b6 100644
--- a/hald/access-check.c
+++ b/hald/access-check.c
@@ -180,24 +180,24 @@ out:
  * @interface_name: the interface to check for
  *
  * This method determines if a caller is locked out to access a given
- * interface on a given device. A caller is locked out when:
+ * interface on a given device. A caller A is locked out of an
+ * interface IFACE on a device object DEVICE if, and only if 
  *
- * 1. Another caller is holding a lock on the interface on the device
- *    non-withstanding that the caller to check for holds the lock
- *    himself.
+ * 1. Another caller B is holding a lock on the interface IFACE on
+ *    DEVICE and A don't have either a global lock on IFACE or a lock
+ *    on IFACE on DEVICE; or 
  *
- * 2. Another caller is holding the global lock for the interface on
- *    the root computer device object and that other caller has
- *    access to the device in question.
+ * 2. Another caller B is holding the global lock on the interface
+ *    IFACE and B has access to DEVICE and and A don't have either a
+ *    global lock on IFACE or a lock on IFACE on DEVICE.
  *
- *    (In other words, a client Foo can grab a lock on the root
- *    computer device object, but that doesn't mean Foo can lock
- *    other clients out of devices that Foo doesn't have access to.)
- *
- * Specifically a caller is not locked out if he has locked the
- * interface and he is the only one holding the lock. However, if two
- * clients have a lock on a device neither of them can access the
- * device.
+ * In other words, a caller A can grab a global lock, but that doesn't
+ * mean A can lock other clients out of devices that A doesn't have
+ * access to. Specifically a caller is never locked out if he has
+ * locked an interface either globally or on the device in
+ * question. However, if two clients have a lock on a device, then
+ * both can access it. To ensure that everyone else is locked out, a
+ * caller needs to use an exclusive lock.
  * 
  * Returns: TRUE iff the caller is locked out
  */
@@ -213,6 +213,8 @@ access_check_caller_locked_out (CITracker   *cit,
         char **holders;
         char **global_holders;
         HalDevice *computer;
+        gboolean is_locked;
+        gboolean is_locked_by_self;
 
         global_lock_name = NULL;
         holders = NULL;
@@ -234,16 +236,15 @@ access_check_caller_locked_out (CITracker   *cit,
          * assumed to have access to the device since they got to hold
          * the lock in the first place. 
          */
+        is_locked = FALSE;
+        is_locked_by_self = FALSE;
         if (holders != NULL) {
                 for (n = 0; holders[n] != NULL; n++) {
-                        if (strcmp (holders[n], caller_unique_sysbus_name) != 0) {
-                                /* Yup, there's someone else... can't do it Sally */
-                                HAL_INFO (("Caller '%s' is locked out of interface '%s' on device '%s' "
-                                           "because caller '%s' got a lock on the interface on the device",
-                                           caller_unique_sysbus_name,
-                                           interface_name,
-                                           hal_device_get_udi (device),
-                                           holders[n]));
+                        is_locked = TRUE;
+                        if (strcmp (holders[n], caller_unique_sysbus_name) == 0) {
+                                is_locked_by_self = TRUE;
+                                /* this is good enough; we are holding the lock ourselves */
+                                ret = FALSE;
                                 goto out;
                         }
                 }
@@ -251,27 +252,37 @@ access_check_caller_locked_out (CITracker   *cit,
 
         if (global_holders != NULL) {
                 for (n = 0; global_holders[n] != NULL; n++) {
-                        if (strcmp (global_holders[n], caller_unique_sysbus_name) != 0) {
-                                /* Someone else is holding the global
-                                 * lock.. check if that someone actually have
-                                 * access to the device...
-                                 */
+                        if (strcmp (global_holders[n], caller_unique_sysbus_name) == 0) {
+                                /* we are holding the global lock... */
                                 if (access_check_caller_have_access_to_device (cit, device, global_holders[n])) {
-                                        /* They certainly do. Give up. */
-                                        
-                                        HAL_INFO (("Caller '%s' is locked out of interface '%s' on device '%s' "
-                                                   "because caller '%s' got a lock on the global interface and "
-                                                   "have access to the device",
-                                                   caller_unique_sysbus_name,
-                                                   interface_name,
-                                                   hal_device_get_udi (device),
-                                                   global_holders[n]));
+                                        /* only applies if the caller can access the device... */
+                                        is_locked_by_self = TRUE;
+                                        /* this is good enough; we are holding the lock ourselves */
+                                        ret = FALSE;
                                         goto out;
                                 }
+                        } else {
+                                /* Someone else is holding the global lock.. check if that someone
+                                 * actually have access to the device...
+                                 */
+                                if (access_check_caller_have_access_to_device (cit, device, global_holders[n])) {
+                                        /* They certainly do. Mark as locked. */
+                                        is_locked = TRUE;
+                                }
                         }
                 }
         }
 
+        if (is_locked && !is_locked_by_self) {
+                /* Yup, there's someone else... can't do it Sally */
+                HAL_INFO (("Caller '%s' is locked out of interface '%s' on device '%s' "
+                           "because someone else got a lock on the interface on the device",
+                           caller_unique_sysbus_name,
+                           interface_name,
+                           hal_device_get_udi (device)));
+                goto out;
+        }
+
         /* done all the checks so we're not locked out */
         ret = FALSE;