Revision 1e97f0047cd3e2b4639f29469969dc699089ba1c

[Evan Schoenberg]

On Feb 25, 2007, at 10:58 PM, Mark Doliner wrote:

> On Sun, 25 Feb 2007 18:51:57 -0500 (EST), evands wrote
>> -----------------------------------------------------------------
>> Revision: 1e97f0047cd3e2b4639f29469969dc699089ba1c
>> Ancestor: 238117a5836fa4e00ce2917d0ac5878906ebb38f
>> Author: evands at pidgin.im
>> Date: 2007-02-25T23:50:54
>> Branch: im.pidgin.pidgin
>>
>> Modified files:
>>         libpurple/dnssrv.c
>>
>> ChangeLog:
>>
>> Patch from Graham Booker which ensures that a process forked by
>> dnsserv cleans up afterwards, calling waitpid() to make sure no
>> zombies are left over.
>
> I thought we had code in pidgin/gtkmain.c that took care of that?   
> I think we
> have a signal set up that calls a function anytime a child process  
> dies.  This
> change seems superfluous to me within Pidgin.  Is it needed in  
> Adium or
> Pidgin-text?

There is indeed code in gtkmain.c which makes this technically not  
needed; it's the clean_pid() function.

There is currently a similar function in Adium's implementation...  
which was added only after several versions were released in which  
there was an extremely mysterious growth over time of zombie  
processes.  I'm not aware of any documentation stating, "Gaim's  
implementation of child processes is such that code elsewhere must be  
careful to clean up after them using a zombie reaper."

Good contract-based programming practices, in my humble opinion,  
demand that code should be locally independent wherever possible, by  
which I mean that if at all feasible a given module should not create  
demands of code elsewhere, especially as regards implementation- 
specific details (a UI developer should not need to go look at the  
source of each file, see if there are any fork() uses which don't  
clean up after themselves, then account for it).

The change in question means that, in the case of dnssrv, no clean_pid 
() style function is necessary for the fork() based implementation;  
if there's a zombie, dnssrv itself takes care of it, and if there's  
not, there's no harm done (since waitpid() returns immediately).  In  
the case of Pidgin, clean_pid() is still needed because apparently  
GStreamer isn't as polite as I'd like libpurple to be, and cleaning  
up after forked'd processes is needed.  In the case of Adium, this  
change now means that every library we're using at present is self- 
contained, and no such function is needed.

Of course, when we go all voice-video, we'll be using GStreamer,  
too... (I hadn't known until just now when I looked at gtkmain.c that  
there was a GStreamer related problem with such things).  This means  
that we'll end up needed a clean_pid(), as well... but that doesn't  
change my original claim: it's just good practices for code to be  
self-contained.

Thoughts?

-Evan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://pidgin.im/pipermail/commits/attachments/20070226/e6ed8f09/attachment.htm