xfce-mcs-manager -> suggest signal poll interval == 1s

[Brian J. Tarricone]

{Sorry about a possible dupe; I sent the original from the wrong email
address.}

On Thu, 24 May 2007 21:01:08 +0200 Jasper Huijsmans wrote:

>Brian J. Tarricone wrote:
>> On Wed, 23 May 2007 23:47:53 -0700 Brian J. Tarricone wrote:
>>> Actually, a better fix -- that would eliminate the polling
>>> completely -- would be to have the signal handler write the status
>>> code to a pipe, and then have the glib mainloop watch the pipe.
>>> Pretty sure writing to a pipe is one of the few things 'allowed' in
>>> a signal handler...
>> 
>> And here's a patch.  It compiles; that's all I know.  Presumably it
>> should work too.  If someone wants to test it, I'll commit it if it
>> works (or someone else can).
>> 
>
>Looks a lot like what I did for the panel, but there are a few 
>differences, and I have no idea if they are important, because, 
>naturally, I just copied the code from some example ;-)
>
>
> From mcs-manager to panel:
>
>- Check return value for write.

I thought about that, but what do we do if write() fails?  I think we'd
just end up ignoring the signal.

>- Don't check condition in io watch callback.

Shouldn't matter.  I only specified that I want G_IO_IN events, so
those are the only ones that should arrive.  Just did the comparison as
a sort of sanity check.

>- g_..._read_chars instead of g_..._read, with use of a union to get 
>chars to int value. Looks overly complicated to me, actually... Ah, a 
>comment says there is no ..._read, so this must be old.

Yeah, using just plain _read() with a 4-byte int should be equivalent.

>- read pipe in a while loop, check at the end if status matches 
>G_IO_STATUS AGAIN.

I'm not convinced this is necessary, since I can't see why all 4 bytes
of the data wouldn't be available at that point, but I suppose it
couldn't hurt.  This isn't really a critical piece of code, and if it
fails, it's not really a big deal.

>- Quit if pipe can't be created.

In this case I think that's bad: if the pipe can't be created and we
quit, mcs-manager isn't running, which sucks.  If we don't quit, then
the only problem (assuming pipe() failing isn't a harbinger of larger
problems) is that mcs-manager won't respond to signals.  Not a big
deal, IMHO.

>- Set reading and writing end of the pipe specifically to non-blocking
>mode.

This is probably a good idea for the writing end -- if we receive a
bunch of signals and the pipe fills before the main loop runs our io
watch function, the signal handler could block forever on write() -- but
shouldn't be necessary for the reading end (actually, not setting
nonblock for the reading end should make the G_IO_ERROR_AGAIN stuff
above unnecessary... I think).  I can't see how the pipe would 'lose'
data before the io watch function is run.

>- Use G_IO_IN | G_IO_PRI instead of just G_IO_IN.

Dunno; can't hurt to add it.  Should modify or remove condition check
in the io watch function.

>- Don't unref channel.

g_io_add_watch() takes a reference on the channel, making the refcount
2.  I unref the channel, bringing it to 1.  Down after gtk_main(), when
I call g_source_remove() to ditch the io watch, it decrements refcount
to 0 and the io channel is freed.

Thanks for the comments -- I wish I'd read your email about already
having that in the panel before I did my own implementation.

And ok, since I don't want to start working on something before I leave
for lunch in a few minutes, I worked up a new patch.  Again, only
compile-tested.

(I need to copy this to xfdesktop too... it's still doing unsafe stuff
in the signal handler.)

	-brian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcs-manager-nopoll-v2.diff
Type: text/x-patch
Size: 4068 bytes
Desc: not available
URL: <http://mail.xfce.org/pipermail/xfce4-dev/attachments/20070524/60b646d0/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.xfce.org/pipermail/xfce4-dev/attachments/20070524/60b646d0/attachment.pgp>