Bug#512839: Your 'xine-ui' NMU introduced new RC bug #512839

2009-01-30 Thread Ben Hutchings
On Fri, 2009-01-30 at 12:28 +0200, Jonathan Quick wrote:
 (re-sending with subject)
 
 Hi Ben
 
 I assume you're aware of the RC bug #512839 introduced by your NMU (and in
 testing) xine-ui/0.99.5+cvs20070914-2.1 ?

No, I missed that.  The bug is clearly not grave; not sure whether it
should be RC.

 Looks like the fork()ing of xdg-screensaver needs a little more thought.

I'm afraid I was a bit lazy about this.  The previous code which forked
gnome-screensaver-command did not close fds either, so I assumed that
would be fine, but since xdg-screensaver's child processes hang around
it's not.

The correct thing to do is to close all fds above 2 before calling
execve() in the child process.

Ben.



signature.asc
Description: This is a digitally signed message part


Bug#512839: Your 'xine-ui' NMU introduced new RC bug #512839

2009-01-30 Thread Ben Hutchings
gark...@mailueberfall.de wrote
 I understand that you don't agree with the bug being grave. The point is
 that I use it mainly for watching TV.
 This is practically not possible anymore now since the VDR playback
 breaks on every channel switch due to xdg-screensaver hogging the audio
 handle. For that usage it is definately a grave bug because it rendered
 my main application useless.
 I know this may not affect all users, but I also find it a bit harsh to
 say a bug is clearly not grave just because one is mainly using a xine
 plugin that has been effectively broken now.

I sympathise, but the definition is:

grave
makes the package in question unusable or mostly so, or causes
data loss, or introduces a security hole allowing access to the
accounts of users who use the package.

I think that this bug is:

important
a bug which has a major effect on the usability of a package,
without rendering it completely unusable to everyone.

but it's up to the maintainer to decide.

 Just as a remark regarding your intended fix: I haven't yet seen the
 place where xine does a wait() or waitpid() for those forked childs to
 clean up the zombies. If there is a place, ok, I just havn't seen it
 yet.

I added signal(SIGCHLD, SIG_IGN) which should make that unnecessary.

Also, the change I actually made to xine-ui *does* include a loop to
close fds 3-255 (sadly there is no easy way to find out what the last
open fd is).  So unless xine is opening a *huge* number of files I can't
understand this bug.

Please could you run ls -l /proc/$pid/fd/ for the pid of the
xdg-screensaver process when this bug is triggered?

 If I startup the XVDR plugin, I see three pid's of xdg-screensaver. Upon
 each TV channel switch, there's one more. They also don't seem to
 terminate, so the list is growing. Aren't they supposed to just signal a
 suspend and then terminate themselves?

Which version of xdg-utils do you have installed?  There was a bug in
that which could result in zombie processes, but I fixed that before
making this change.

Ben.



signature.asc
Description: This is a digitally signed message part


Bug#512839: Your 'xine-ui' NMU introduced new RC bug #512839

2009-01-30 Thread Ben Hutchings
On Sat, 2009-01-31 at 02:25 +, Ben Hutchings wrote:
  Just as a remark regarding your intended fix: I haven't yet seen the
  place where xine does a wait() or waitpid() for those forked childs to
  clean up the zombies. If there is a place, ok, I just havn't seen it
  yet.
 
 I added signal(SIGCHLD, SIG_IGN) which should make that unnecessary.
 
 Also, the change I actually made to xine-ui *does* include a loop to
 close fds 3-255 (sadly there is no easy way to find out what the last
 open fd is).  So unless xine is opening a *huge* number of files I can't
 understand this bug.

OK, now I see what's happened.  The correct fix for #374644 is in sid,
but not in lenny.  Because the new version of xine-ui in sid changed
library dependencies, it could not propagate to lenny and I had to
upload separately to testing-proposed-updates.  I mistakenly uploaded an
earlier version of the changes which has this bug.  I will upload a new
version to t-p-u.

Ben.



signature.asc
Description: This is a digitally signed message part