evan at pidgin.im
Mon Feb 26 06:21:30 EST 2007
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:
>> 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
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
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Commits