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: --- dvbdevice.c~ 2007-12-10 15:19:51.116943936 +0100 +++ dvbdevice.c 2007-12-10 15:19:51.120944682 +0100 @@ -63,6 +63,7 @@ int fd = open(FileName, Mode); if (fd < 0 && ReportError) LOG_ERROR_STR(FileName); + fcntl(fd, F_SETFD, FD_CLOEXEC); return fd; }
--- svdrp.c~ 2007-12-10 15:20:12.476929058 +0100 +++ svdrp.c 2007-12-10 15:20:12.480929804 +0100 @@ -91,7 +91,7 @@ LOG_ERROR; return false; } - oldflags |= O_NONBLOCK; + oldflags |= O_NONBLOCK|FD_CLOEXEC; if (fcntl(sock, F_SETFL, oldflags) < 0) { LOG_ERROR; return false;
Comments, ideas? I would be happy to see this little patch applied to 1.4 and 1.5 trunks of VDR.
Deti
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
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
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 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
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
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...
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
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
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
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.
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
-
Anssi Hannula -
Darren Salt -
Deti Fliegl -
Klaus Schmidinger