pm-utils does not follow requests to inhibit suspend/hibernate from scripts in /etc/pm/sleep.d

Bug #665651 reported by Paulo J. S. Silva on 2010-10-23
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pm-utils
Confirmed
High
pm-utils (Debian)
Fix Released
Unknown
pm-utils (Ubuntu)
High
Brian Murray

Bug Description

Binary package hint: pm-utils

1) ubuntu version

Description: Ubuntu 10.10
Release: 10.10

2) package version:

pm-utils:
  Instalado: 1.4.1-3
  Candidato: 1.4.1-3
  Tabela de versão:
 *** 1.4.1-3 0
        500 http://br.archive.ubuntu.com/ubuntu/ maverick/main amd64 Packages
        100 /var/lib/dpkg/status

3) If a script in /etc/pm/sleep.d returns a non-zero value upon requests to suspend or hibernate, the suspend/hibernate process should be aborted.

4) The system ignores the return value and supends/hibernates anyhow.

In order to reproduce the bug just drop the 00_avoid_suspend script in /etc/pm/sleep.d, make it executable and try to sleep.
The system should refuse to sleep but, at least here, it doesn't.

The problems seems to be in the _run_hook() function defined in file /usr/lib/pm-utils/pm-fuctions. This is the function that calls the scripts in the /etc/pm/sleep.d directory. After calling the script, _run_hook calls the function log twice before checking the script return value. It seems that the return value is being set back to 0 in these calls to log (but I am no bash expert).

To walk around I just added a line to store the script return value in a temporary variable and used that variable after the log calls to check the original result. Now it seems to work. My current _run_hook function looks like this:

_run_hook() {
 # $1 = hook to run
 # rest of args passed to hook unchanged.
 log "Running hook $*:"
 hook_ok "$1" && "$@"
 status=$?
 log ""
 log -n "$*: "
 hook_exit_status $status && LAST_HOOK="${1##*/}" || inhibit
}

Since I am no bash expert, I can not assure that this is a proper solution, but it works here.

Created attachment 40891
patch for pm-functions.in

Since 1.4.0, _run_hook calls a log() after running the hook, and thus loses the hook exit status:

199 hook_ok "$1" && "$@"
200 log ""
201 log -n "$*: "
202 hook_exit_status $? && LAST_HOOK="${1##*/}" || inhibit

I attached a simple patch that seems to fix it.

This is important because e.g. a script that unmounts SD/MMC cards before suspending cannot inhibit it if the card is busy (see https://help.ubuntu.com/community/AspireOne/Ubuntu9.10?action=recall&rev=69).

Regards,
  Ariel

Umm, I hoped there would be some kind of reply by now. Should I learn git and try to submit the patch there?

Peter Wu (lekensteyn) wrote :

hook_exit_status relies on the last exit code, but the function `log()` always returns 0.

The log lines adds something like:
"/usr/lib/pm-utils/sleep.d/000kernel-change hibernate hibernate:
and hook_exit_status:
 success.

Shouldn't the log lines be moved to hook_exit_status? Otherwise, the exitcode should be stored in a variable and applied to hook_exit_code as described in the bug report.

This bug does not occur at Debians pm-utils package, there are no log lines between hook_ok and hook_exit_code

Changed in pm-utils (Ubuntu):
status: New → Confirmed
Peter Wu (lekensteyn) wrote :

If we modify log() to make it return the previous error code, the following code are affected:

sleep.d/75modules
* resume_modules() calls modreload(), if it fails to load a module, the return value of sleep.d/75modules will match the return value of modprobe (is this the expected behavior, or should resume_modules return 0 just like suspend_modules? I vote for the latter one)

pm-functions
* load_hook_blacklist(): if disablehook() fails to write to ${STORAGEDIR}/disable_hook:HOOKNAME, the return value of load_hook_blacklist will be the return value of disablehook() (this is unlikely to happen, but should load_hook_)
* _run_hook: this function will work as expected as the return value of hook_ok propagates to hook_exit_status

Other functions/ scripts are not affected by this change in log() . Should log() change the previous exitcode or not?

The patch provided does not change the behavior of pm-utils, other than making _run_hook() work as it should do.

tags: added: patch
Disidente (disidente) wrote :

Had the same problem in xubuntu 11.04.

Published patch corrects pm-utils behaviour. I use a pm hook to umount SD cards on suspend, in order to avoid data corruption on resume (as described on https://bugs.launchpad.net/ubuntu/karmic/+source/linux/+bug/424877).

My hook looks like this:
#!/bin/sh
# Drop to: /etc/pm/sleep.d
# Use this script to prevent data loss on gnome-mounted MMC/SD
# cards. It syncs data and umounts all mmcblk devices prior to
# suspend, and cancels suspend if umounting was not posssible
# (i.e: something locks a file)
case "$1" in
 hibernate|suspend)
  sync
  for drive in $( ls /dev/mmcblk?p* ); do
      umount ${drive} > /dev/null
      # If umount failed: abort suspend
      if [ $? -gt 0 ]; then
   # Test if device keeps mounted. Previous command could fail
   # (i.e device was not mounted) with a non-stopper
   # problem for the suspend precess
   mount | grep ${drive}
   if [ $? -eq 0 ]; then
       echo "Failed to umount ${drive}" > /tmp/suspend.log
       exit 1
   fi
      fi
  done
  ;;
esac

My hook doesn't work without published patch, and without this hook data corruption is possible... so, I vote for high importance on this bug.

Paulo J. S. Silva (pjssilva) wrote :

Please, some one pass on the patch upstream. Or should I try to do it?

This bug have almost 8 months, and there is a patch that works. Please fix it. I got bitten by it once again after upgrading to 11.04...

Peter Wu (lekensteyn) wrote :

I've currently have the next command in my post-installation script for fixing the bug:

tmp="$(mktemp)"&&wget https://launchpadlibrarian.net/73767502/pm-functions-run-hooks-with-local.patch -O"$tmp"&&\
echo "8b59fdcb8f79c40510c15f6a8f3695f9569b084c6f6640e8175cf3105d81439e $tmp"|sha256sum -c&&\
sudo patch /usr/lib/pm-utils/pm-functions "$tmp";rm $tmp

The upstream bug has been reported two months after this one: https://bugs.freedesktop.org/show_bug.cgi?id=32210

Changed in pm-utils:
importance: Unknown → High
status: Unknown → Confirmed
Changed in pm-utils (Ubuntu):
status: Confirmed → Triaged
importance: Undecided → High
tags: added: oneiric
Brian Murray (brian-murray) wrote :

With the patched version of pm-utils and the test case provided I see the following in /var/log/pm-suspend.log:

Running hook /etc/pm/sleep.d/05_avoid-sleep suspend suspend:

/etc/pm/sleep.d/05_avoid-sleep suspend suspend: Returned exit code 1.
Fri Jul 8 15:29:57 PDT 2011: Inhibit found, will not perform suspend

Changed in pm-utils (Ubuntu):
status: Triaged → In Progress
assignee: nobody → Brian Murray (brian-murray)
Brian Murray (brian-murray) wrote :

Attached is a debdiff that resolves this issue in Oneirc. It might be worth SRU'ing to stable releases of Ubuntu too.

Daniel T Chen (crimsun) on 2011-07-08
tags: added: patch-forwarded-debian
removed: patch
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package pm-utils - 1.4.1-8ubuntu1

---------------
pm-utils (1.4.1-8ubuntu1) oneiric; urgency=low

  * Add 26-inhibit-on-right-status.patch: Do not use the exit status of log
    rather the exit status of the hook thereby allowing inhibit to work.
    Thanks to Launchpad user Lekensteyn for the patch. (LP: #665651)
 -- Brian Murray <email address hidden> Fri, 08 Jul 2011 15:02:58 -0700

Changed in pm-utils (Ubuntu):
status: In Progress → Fix Released
Brian Murray (brian-murray) wrote :

Well this ended up revealing an error in /etc/pm/sleep.d/novatel_3g_suspend provided by toshset and preventing suspend in Oneiric. So one should be extra cautious if SRU'ing this fix as it may break suspend and hibernate.

Changed in pm-utils (Debian):
status: Unknown → New
Changed in pm-utils (Debian):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.