I know, HAL is everywhere, and sudo fallback is considered unmaintained. Although having it not working fucks up OpenBSD/NetBSD/Solaris users, and maybe other linux distribs (i know slackware provides HAL only since its last version,.) So i dig in its code, and here are my findings : - xfce4-session-logout --reboot only logs you out (ditto when clicking on reboot in window) - unrelated, but the code in xfsm-shutdown-helper.c used to ask password to the user seems unused. When you don't have NOPASSWD set in sudoers, only LogOut option is available, others are greyed. Maybe those chunks of code can be removed/marked deprecated - echo 'REBOOT' | sudo /usr/local/libexec/xfsm-shutdown-helper does its job. So xfce4-session doesn't send him the good command -> bingo. There's an offset mistake in xfsm-shutdown-helper.c: xfsm_shutdown_helper_send_command(), introduced in r28049. it receives command, which is now the enum defined in shutdown.h, 1=logout, 2=halt, 3=reboot.. but command_table[command] is wrong, as 0='POWEROFF' and 1='REBOOT'. (which previously corresponded to XFSM_SHUTDOWN_POWEROFF = 0 / XFSM_SHUTDOWN_REBOOT = 1 in xfsm-shutdown-helper.h) The simplest fix (to me) is to have command_table[command - 2], or this dumb snippet : if (command == XFSM_SHUTDOWN_HALT) fprintf (helper->outfile, "POWEROFF\n"); else fprintf (helper->outfile, "REBOOT\n"); At some point, suspend/resume can be supported in this sudo fallback too (as it was done previously), no need to _only_ rely on HAL. If needed, i can take care of the maintenance of these parts.. As a side note, you added some error reporting through dialogs.. it sometimes happen to me to have (after session is saved and all windows closed) 'Error receiving response from shutdown helper: interrupted system call' dialog.. which may make sense, as the system is shutting down, there might be a race condition with the helper sending back 'SUCCEED', and fgets() in xfce4-session. Thoughs ? Is my reasoning correct ?
(In reply to comment #0) > There's an offset mistake in xfsm-shutdown-helper.c: > xfsm_shutdown_helper_send_command(), introduced in r28049. it receives command, > which is now the enum defined in shutdown.h, 1=logout, 2=halt, 3=reboot.. but > command_table[command] is wrong, as 0='POWEROFF' and 1='REBOOT'. (which > previously corresponded to XFSM_SHUTDOWN_POWEROFF = 0 / XFSM_SHUTDOWN_REBOOT = > 1 in xfsm-shutdown-helper.h) > > The simplest fix (to me) is to have command_table[command - 2] Do you have a patch? > At some point, suspend/resume can be supported in this sudo fallback too (as it > was done previously), no need to _only_ rely on HAL. If needed, i can take care > of the maintenance of these parts.. Yeah, that would be fine, for 4.8. > As a side note, you added some error reporting through dialogs.. it sometimes > happen to me to have (after session is saved and all windows closed) 'Error > receiving response from shutdown helper: interrupted system call' dialog.. > which may make sense, as the system is shutting down, there might be a race > condition with the helper sending back 'SUCCEED', and fgets() in xfce4-session. > > Thoughs ? Is my reasoning correct ? Hmm, yeah, that could be the problem. It's probably safe to ignore EINTR at that point.
Created attachment 2110 Get the right offset in command_table, and ignore EINTR Here you are, fixes the issue for me.
Created attachment 2141 Fixes sudo fallback helper whoops, wrong patch.
Committed, thanks. (In the future, please, one bug report and patch per issue.)
A crash still happens in this fix (commit 588bcec67ca5aba6a91905ce589cce2283c5a9e5) : if fgets() or ferror() fail in xfsm-shutdown-helper.c:xfsm_shutdown_helper_send_command() and errno is set to EINTR, false is returned but error is not set. Then, in shutdown.c:xfsm_shutdown(), xfce_message_dialog() is called with error->message, causing a NULL dereference. Two options to fix this crash : - in xfsm_shutdown(), call xfce_message_dialog() only if error is set. quite ugly. - in xfsm_shutdown_helper_send_command(), move the 'return FALSE' statement _inside_ the check for 'error && errno != EINTR'. I'll tend to prefer the latter.
Ok, ended up just returning true on the spot if it's EINTR. Pushed to master and 4.6 branch.
er, mark fixed again.