After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 590245 - [regression] 'evolution --force-shutdown' should kill factories
[regression] 'evolution --force-shutdown' should kill factories
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Contacts
2.28.x (obsolete)
Other Linux
: Normal major
: ---
Assigned To: evolution-addressbook-maintainers
Evolution QA team
evolution[dbus]
Depends on:
Blocks:
 
 
Reported: 2009-07-30 10:40 UTC by Akhil Laddha
Modified: 2010-12-07 14:41 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
eds patch (2.49 KB, patch)
2010-12-07 14:33 UTC, Milan Crha
committed Details | Review
evo patch (3.67 KB, patch)
2010-12-07 14:38 UTC, Milan Crha
committed Details | Review

Description Akhil Laddha 2009-07-30 10:40:16 UTC
Evolution 2.27.5 + dbus-hybrid branch 

Currently it doesn't kill e-addressbook-factory process.
Comment 1 Matthew Barnes 2009-07-30 12:46:40 UTC
Are we sure we still want --force-shutdown to be this destructive?
Comment 2 Chenthill P 2009-07-31 05:21:57 UTC
We suggest it to users only in cases where eds hangs up for some reasons. So it has to be working the same way.
Comment 3 Ross Burton 2009-10-02 11:52:29 UTC
It should also kill e-calendar-factory.
Comment 4 Matthew Barnes 2009-10-02 12:17:52 UTC
Is there a more graceful way to stop these factory processes than by just killing them?  I'm thinking give them a chance to exit gracefully in case they're -not- hung, before we go resorting to violence.
Comment 5 Ross Burton 2009-10-02 14:09:19 UTC
Not at the moment.  We could use sigusr1 or something?

Actually, Maemo has a patch I wrote where you sigusr1 the e-addressbook-factory, and it invokes the new "flush" method on every open backend, which the file backend implements and writes its current state to disk.  Maemo uses it when starting the backup to ensure that the on-disk state is up to date.

We could use this code.  It's not perfect -- how long do you want after sending sigusr1 before killing -- but it works. Alternative extended it so that sigusr2 does a flush and then exists when the flushing is complete, so force-shutdown can send sigusr2 and then wait a few seconds before sending sigkill.
Comment 6 Matthew Barnes 2009-10-02 17:10:38 UTC
What about just building a "shutdown" method into the D-Bus API?  (But don't necessarily wrapper it in the E-D-S libraries.)

Server side would acknowledge the command immediately and return its PID (or can you already get that through libdbus?).

Client side would wait a brief period for the acknowledgement, then once received give the server significantly longer to finish flushing and terminate before sending SIGKILL.  (I assume a hang while flushing would be pretty rare.)  If we don't get an acknowledgement back before the first timeout, it's a good bet the server is hung and we can kill it immediately.

Wasn't sure if D-Bus gives you anything like this natively.
Comment 7 Matthew Barnes 2009-10-02 17:16:17 UTC
Would also be nice for --force-shutdown to indicate whether each process exited cleanly or had to be killed.  Useful info to ask for in bug reports.
Comment 8 Milan Crha 2010-12-07 14:33:43 UTC
Created attachment 176009 [details] [review]
eds patch

for evolution-data-server;

Not so great, but till someone comes with something better. Just send the -QUIT signal to the factory and it'll exit. Though it'll just exit and nothing else.
Comment 9 Milan Crha 2010-12-07 14:38:07 UTC
Created attachment 176013 [details] [review]
evo patch

for evolution;

Also listens for -QUIT signal, and the killev sends it to factories and evo (just in case). killev is also so clever that if not run under X then it sends a -QUIT instead of invoking evolution. Also, the 'evolution --force-shutdown' can be run without X now.

There are still some places for improvements, but as nobody cared enough for more than a year, then I think this is better than nothing.
Comment 10 Milan Crha 2010-12-07 14:41:54 UTC
Created commit 9ea37a5 in eds master (2.91.4+)
Created commit 7ddb393 in evo master (2.91.4+)