Re: [Rhythmbox-devel] proper shutdown on ctrl-c and other stuff



On Sat, 2004-02-14 at 22:43, Andreas Bombe wrote:
> Currently rb seems to have no signal handler to catch SIGINT or
> anything.  This means that it dies hard when it gets any termination
> signal without shutting down properly.

Signals are basically evil, and interact especially poorly with
multithreaded applications.  

One nice thing about Rhythmbox being based purely GTK+ and GNOME is that
in theory we can be portable to other platforms like MacOS and Windows,
the latter of which doesn't have signals.

So I'd rather avoid sprinking #ifdefs about the source and the
portability problems that go with that, just to get a nice shutdown if
you send SIGINT (which really no normal user should be doing).

If you wanted to do this really right, probably add a patch to GTK+ to
intercept SIGINT itself, and optionally run a callback in the
application.  Like g_main_context_set_system_interrupt_handler (...).

> I can't see an obvious way to shut down or signal to shut down the
> running RBShell in the proper context.  

Doing anything nontrivial from a signal handler context is a big pain. 
It might work if you queued an idle function from the handler, but even
that is kind of shady if the SIGINT came inside the glib code for
queueing a signal handler - it would probably just deadlock, since it
would try to re-acquire the already locked context mutex.

The safest way to do it would be to set a simple global variable, and
have an idle function that polls that every 2 seconds or something.

But again, let's just not do it :)  It's not worth it.

> Something different:  The GNOME icon may be set from a NULL pointer.
> Granted, only in incomplete installations but it still isn't nice.

Ok, merged, thanks:

* committed rhythmbox-devel@gnome.org--2004/rhythmbox--main--0.7--patch-85

Ideally we'd be able to work completely uninstalled, but I think that
requires some additions to the GNOME library APIs.

> Since I didn't get further with the signal stuff this is also the only
> patch in aeb@debian.org--2004/rhythmbox--main--0.7 (to be found on
> http://home.in.tum.de/~bombe/arch/aeb@debian.org--2004 )

Cool, I added this archive to the development page.

> Also, the sources are riddled with whitespace at end of lines,
> whitespace only lines and inconsistently mixed spaces/tabs.  Sic an
> Emacs with develock mode on the sources and have your eyeballs melt.  I
> suppose fixing that would be very intrusive, resulting in many merge
> rejects and the like.

Right...at least the mixture of tabs and spaces I suppose is mostly my
fault, since I use Emacs (which defaults to that), but I inherited the
original codebase from a vim user.

There's really two ways to solve this - tabs only, or spaces only.  If
we're going to change, I'd like to just use spaces only, since it allows
for nicer-looking indentation.  When people submit patches or make
commits, it'd be fine to also change that region of code to spaces
only.  You're right in that we shouldn't try to do it all at once.

I'll add the requisite Emacs magic to make Emacs do spaces-only.  Does
anyone know the vim magic to do the same?

This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]