Re: [vdr] [patch] avoid inheritance of file descriptors
On 12/11/2007 01:09 PM, Darren Salt wrote: > I demand that Klaus Schmidinger may or may not have written... > > [snip; FD_CLOEXEC] >> Why is this suddenly such a big problem? >> If a plugin wants to run an external program it can simply use >> SystemExec(). > >> Besides, as Darren Salt pointed out, this flag is apparently only available >> in the very latest kernel version, which I'm pretty sure is not that widely >> in use yet. > > Er, no: FD_CLOEXEC has been available for years (see fcntl(2)). You're > confusing it with O_CLOEXEC, which is new. Oh, sorry. Nevertheless, SystemExec() (or something similar) is what a plugin should use when launching an external program. Klaus ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
I demand that Klaus Schmidinger may or may not have written... [snip; FD_CLOEXEC] > Why is this suddenly such a big problem? > If a plugin wants to run an external program it can simply use > SystemExec(). > Besides, as Darren Salt pointed out, this flag is apparently only available > in the very latest kernel version, which I'm pretty sure is not that widely > in use yet. Er, no: FD_CLOEXEC has been available for years (see fcntl(2)). You're confusing it with O_CLOEXEC, which is new. -- | Darren Salt| linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Burn less waste. Use less packaging. Waste less. USE FEWER RESOURCES. I am Bjorn of Borg. Wimbledon is irrelevant. ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
On 12/11/2007 08:39 AM, Deti Fliegl wrote: > Klaus Schmidinger wrote: >> Plugins can call SystemExec() just as well when the want to execute >> an external program. > Yes, but would you point every single developer on this issue? > >> IMHO it is no feasible solution to expect every file handle to >> be opened with FD_CLOEXEC. Even if VDR itself would do this, there >> could still be plugins that don't. > It is enough to let VDR set this flag on its own file handles - the main > problem is that while an external script/program is running and because > of inherited DVB handles > - zapping is blocked > - a restart of VDR is impossible. > >> I'd say if some plugin wants to run an external program, it needs >> to take care by itself that all unneeded file handles are closed. >> That's what SystemExec() is for. > Whatever - if you don't see the point I cannot help. Why is this suddenly such a big problem? If a plugin wants to run an external program it can simply use SystemExec(). Besides, as Darren Salt pointed out, this flag is apparently only available in the very latest kernel version, which I'm pretty sure is not that widely in use yet. Klaus ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
Klaus Schmidinger wrote: > Plugins can call SystemExec() just as well when the want to execute > an external program. Yes, but would you point every single developer on this issue? > IMHO it is no feasible solution to expect every file handle to > be opened with FD_CLOEXEC. Even if VDR itself would do this, there > could still be plugins that don't. It is enough to let VDR set this flag on its own file handles - the main problem is that while an external script/program is running and because of inherited DVB handles - zapping is blocked - a restart of VDR is impossible. > I'd say if some plugin wants to run an external program, it needs > to take care by itself that all unneeded file handles are closed. > That's what SystemExec() is for. Whatever - if you don't see the point I cannot help. Deti ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
I demand that Deti Fliegl may or may not have written... > Anssi Hannula wrote: >> Deti Fliegl wrote: >>> Klaus Schmidinger wrote: Doesn't SystemExec() (see tools.c) take care of this? >>> Yes you are right - it takes care internally but not for plugins like >>> dvdswitch etc. In order to fix this problem you could patch every single >>> plugin or just set the right file descriptor flag once. I think the >>> latter does not cause any interference and should solves some issues. >> For the record, the latter creates a small race condition: an external >> program could be launched before FD_CLOEXEC is set on an fd. > In VDR all file descriptors seem to be allocated at startup. IMHO it is > not very likely that a race condition could happen. >From open(2): O_CLOEXEC (Since Linux 2.6.23) Enable the close-on-exec flag for the new file descriptor. Specifying this flag permits a program to avoid an additional fcntl(2) F_SETFD operation to set the FD_CLOEXEC flag. Addi- tionally, use of this flag is essential in some multithreaded programs since using a separate fcntl(2) F_SETFD operation to set the FD_CLOEXEC flag does not suffice to avoid race condi- tions where one thread opens a file descriptor at the same time as another thread does a fork(2) plus execve(2). No use *now*, I know - too many people not yet using a new-enough kernel... -- | Darren Salt| linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | Kill all extremists! Vote Anarchist. ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
On 12/10/07 21:07, Deti Fliegl wrote: > Klaus Schmidinger wrote: >> Doesn't SystemExec() (see tools.c) take care of this? > Yes you are right - it takes care internally but not for plugins like > dvdswitch etc. In order to fix this problem you could patch every single > plugin or just set the right file descriptor flag once. I think the > latter does not cause any interference and should solves some issues. Plugins can call SystemExec() just as well when the want to execute an external program. IMHO it is no feasible solution to expect every file handle to be opened with FD_CLOEXEC. Even if VDR itself would do this, there could still be plugins that don't. I'd say if some plugin wants to run an external program, it needs to take care by itself that all unneeded file handles are closed. That's what SystemExec() is for. Klaus ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
Deti Fliegl wrote: > Anssi Hannula wrote: >> Deti Fliegl wrote: >>> Klaus Schmidinger wrote: Doesn't SystemExec() (see tools.c) take care of this? >>> Yes you are right - it takes care internally but not for plugins like >>> dvdswitch etc. In order to fix this problem you could patch every single >>> plugin or just set the right file descriptor flag once. I think the >>> latter does not cause any interference and should solves some issues. >> For the record, the latter creates a small race condition: an external >> program could be launched before FD_CLOEXEC is set on an fd. > In VDR all file descriptors seem to be allocated at startup. IMHO it is > not very likely that a race condition could happen. ... except on dvr devices, I know. Deti ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
Anssi Hannula wrote: > Deti Fliegl wrote: >> Klaus Schmidinger wrote: >>> Doesn't SystemExec() (see tools.c) take care of this? >> Yes you are right - it takes care internally but not for plugins like >> dvdswitch etc. In order to fix this problem you could patch every single >> plugin or just set the right file descriptor flag once. I think the >> latter does not cause any interference and should solves some issues. > > For the record, the latter creates a small race condition: an external > program could be launched before FD_CLOEXEC is set on an fd. In VDR all file descriptors seem to be allocated at startup. IMHO it is not very likely that a race condition could happen. Deti ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
Deti Fliegl wrote: > Klaus Schmidinger wrote: >> Doesn't SystemExec() (see tools.c) take care of this? > Yes you are right - it takes care internally but not for plugins like > dvdswitch etc. In order to fix this problem you could patch every single > plugin or just set the right file descriptor flag once. I think the > latter does not cause any interference and should solves some issues. For the record, the latter creates a small race condition: an external program could be launched before FD_CLOEXEC is set on an fd. -- Anssi Hannula ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
Klaus Schmidinger wrote: > Doesn't SystemExec() (see tools.c) take care of this? Yes you are right - it takes care internally but not for plugins like dvdswitch etc. In order to fix this problem you could patch every single plugin or just set the right file descriptor flag once. I think the latter does not cause any interference and should solves some issues. Deti ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] [patch] avoid inheritance of file descriptors
On 12/10/07 15:28, Deti Fliegl wrote: > Hi, > > I think there is a problem in calling external programs from plugins. If > such a program takes some while for execution (even in background) it > gets inherited all file descriptors of VDR. This prevents vdr from > zapping to another channel or even from restarting properly. You will > see messages like: > > ERROR: /dev/dvb/adapter0/dvr0: Device or resource busy > or > ERROR (svdrp.c,84): Address already in use > > Reason: By default unix inherits all file descriptors to child processes > when calling exec*(...) or system(...). You can avoid this by setting > FD_CLOEXEC on all file descriptors that should not be inherited. > > Patch: > ... > Comments, ideas? Doesn't SystemExec() (see tools.c) take care of this? Klaus ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
