! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Fix some bit in XfceSmClient
Status:
RESOLVED: FIXED

Comments

Description Nick Schermer editbugs 2010-05-10 08:03:04 CEST
I think this is not fully tested yet. Most important is that the "quit" signal needs to be implemented, so the application is not terminated. The argv is also not supplied to the client (to handle --sm-client-id), look at xfce4-panel or xfdesktop how this is done.
Comment 1 Brian J. Tarricone (not reading bugmail) 2010-05-10 09:12:35 CEST
Just FYI: not sure about the need to implement ::quit.  If the app has already saved its state (which it should have in response to ::quit-requested or ::save-state), it's fine if the ::quit signal causes the app to terminate without the app getting a notification.

There are potentially some cases when an app might receive ::quit out of the blue (without a preceding ::save-state or ::quit-requested), but those should be rare, and, regardless, there isn't much it can do about it: it is *required* to quit in response to the signal (in fact, I toyed with the idea of putting an exit(0) after the signal emission).  If it doesn't, likely the app will get killed anyway.

At any rate, if you want to try to cancel a shutdown or avoid being terminated, the place to do so is in the ::quit-requested handler, and, even then, the session manager is allowed to ignore you and make you quit anyway.  The WM is not at all special in this regard.

I think the g_critical() added to xsmp_die() is a bit alarmist and incorrect.  At best I would suggest removing it entirely; at worst it should be downgraded to a g_message() or even g_debug().  The intent there is that, as a convenience, apps can ignore ::quit if they don't care (which should be the common case!).  Printing a critical message at that point is just console spam and will probably just encourage well-meaning but uninformed users to file a bug against the app.

But your point about needing to either use xfce_sm_client_get_option_group() or _new_with_argv() (the former preferred) on startup is of course valid!
Comment 2 Brian J. Tarricone (not reading bugmail) 2010-05-10 09:17:43 CEST
P.S.  The only reason xfdesktop connects to ::quit is to ask the main loop to terminate, so it can free memory at the bottom of main().  That's probably only useful when running the app through valgrind, certainly not required, and in fact probably makes shutdown/logout slower!

(And even with valgrind it's of dubious use... I'm not ever aware of a time I've run xfdesktop through valgrind with the intent to test for memory leaks at logout; manually terminating xfdesktop certainly suffices for that test.)
Comment 3 Nick Schermer editbugs 2010-05-10 09:58:28 CEST
Yeah, well handling quit is just 1 line of code to trigger gtk_main_quit. I can change it to a debug message, will do that tonight, but better to leave it at some level, since I think it is something an app should handle (ie save some last bits after gtk_main for example) and people should be aware we terminate (which is uncommon from within a library).
Comment 4 Brian J. Tarricone (not reading bugmail) 2010-05-10 11:52:54 CEST
I'd still urge you to remove it entirely.  It's well documented in the API docs.  It only happens right before the SM would send the process a SIGTERM anyway, so the presence of the signal is merely a courtesy.

Bug #6435

Reported by:
Nick Schermer
Reported on: 2010-05-10
Last modified on: 2010-09-12

People

Assignee:
Olivier Fourdan
CC List:
2 users

Version

Version:
unspecified

Attachments

Additional information