5

I want to run programs in linux by a button click an therefore I wrote a function execute:

void execute(const char* program_call, const char* param )
{
    pid_t child = vfork();

    if(child == 0) // child process
    {
        int child_pid = getpid();

        char *args[2]; // arguments for exec
        args[0] = (char*)program_call; // first argument is program_call
        args[1] = (char*)param;

        // close all opened file descriptors:
        const char* prefix = "/proc/";
        const char* suffix = "/fd/";
        char child_proc_dir[16]; 
        sprintf(child_proc_dir,"%s%d%s",prefix,child_pid, suffix);

        DIR *dir;
        struct dirent *ent;

        if ((dir = opendir (child_proc_dir)) != NULL) {
            // get files and directories within directory
            while ((ent = readdir (dir)) != NULL) {
                // convert file name to int
                char* end;
                int fd = strtol(ent->d_name, &end, 32);
                if (!*end) // valid file descriptor
                {
                    close(fd); // close file descriptor
                    // or set the flag FD_CLOEXEC
                    //fcntl( fd, F_SETFD, FD_CLOEXEC );
                }
            }
            closedir (dir);
        } 
        else 
        {
            cerr<< "can not open directory: " << child_proc_dir <<endl;
        }
        // replace the child process with exec*-function
            execv(program_call,args);
            _exit(2);
        }
    else if (child == -1) // fork error
    {
        if (errno == EAGAIN)
        {
            cerr<<“To much processes"<<endl;
        }
        else if (errno == ENOMEM)
        {
            cerr<<“Not enough space available."<<endl;
        }
    }
    else // parent process
    {
        usleep(50); // give some time 
        if ( errno == EACCES)
        {
            cerr<<“Permission denied or process file not executable."<<endl;
        }
        else if ( errno == ENOENT)
        {
            cerr<<"\n Invalid path or file."<<endl;
        }
        int child_status;
        if ( waitpid(child, &child_status, WNOHANG | WUNTRACED) < 0) // waitpid failed
        {
            cerr<<"Error - Execution failed"<<endl;
        }
        else if ( WIFEXITED( child_status ) &&  WEXITSTATUS( child_status ) != 0)   
        {
            cerr<<“Child process error - Execution failed"<<endl;
        }
    }
}

There are two problems:

  1. Closing the file descriptors causes some problems, for example Thunderbird crashes or VLC runs without sound. More exactly: closing of stdout(1) and stderr(2) causes these problems. As I understand, closing file descriptor before exec only prevents them from been duplicated (there is no need to send informations from child process to parent process). Why does this affect the child process? Replacing close() by setting the flag FD_CLOEXEC doesn't change anything. Also setting the FD_CLOEXEC flag before fork doesn't solve the problem. Is there a better way to prevent inheritance of file descriptors?

  2. The return value of waitpid is often 0, even if the program call fails, I think because there are two (asynchrone) processes. usleep(50) solves this problem for my needs, but I hope there are better solutions for this problem.

I'm using vfork, but the same problems occur by using fork.

BiscuitBaker
  • 1,421
  • 3
  • 23
  • 37
Lexa
  • 51
  • 1
  • 1
  • 4
  • 2
    AFAIK file descriptors are shared between children and parents. You'd have to create a copy of the original descriptor with `dup(2)` in the child process, and use only this copy within your child process. – Philipp Murry Dec 15 '14 at 12:03
  • What is your application doing? What file descriptors is it using? How is your `execute` used? – Basile Starynkevitch Dec 15 '14 at 12:58
  • My application should be a simple start panel for other applications. It is now using no extraneous file descriptors, but this may change later. execute is used in a button callback function (FLTK). @Philipp Murry: What is the advantage to use only the duplicated file descriptors from dup2() ? – Lexa Dec 16 '14 at 11:00
  • With `dup` you would just get a new file descriptor; one that is independent of the first one. It's like calling `open` on the file again. Therefore, closing the first file descriptor does not effect the second (the copy created with `dup`). – Philipp Murry Dec 16 '14 at 11:15

2 Answers2

5

First, in 2014, never use vfork but simply fork(2). (Since vfork(2) is obsolete since POSIX 2001 and removed in POSIX 2008).

Then, the simplest way to close most of file descriptors is just

for (int fd=3; fd<256; fd++) (void) close(fd);

(hint: if a fd is invalid, close(fd) would fail and we ignore the failure; and you start from 3 to keep open 0==stdin, 1==stdout, 2==stderr; so in principle all the close above would fail).

However, well behaved and well-written programs should not need such a loop on closing (so it is a crude way to overcome previous bugs).

Of course, if you know that some file descriptor other than stdin, stdout, stderr is valid and needed to the child program_call (which is unlikely) you'll need to explicitly skip it.

and then use FD_CLOEXEC as much as possible.

It is unlikely that your program would have a lot of file descriptors without you knowing them.

Maybe you want daemon(3) or (as commented by vality) posix_spawn.

If you need to explicitly close STDIN_FILENO (i.e. 0), or STDOUT_FILENO (i.e. 1), or STDERR_FILENO (i.e. 2) you'll better open("/dev/null",... and dup2 them after - before calling exec, because most programs expect them to exist.

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
  • 1
    I would discourage the use of a for loop to close all file descriptors, because you may close valid file descriptors. I have seen code like this to break because somebody opened a file before that loop and used the file descriptor afterwards. would you care to elaborate why you discourage using vfork? – Alexander Oh Dec 15 '14 at 12:49
  • `fork` is quick enough (because of lazy copy-on-write paging). `vfork` is useless. – Basile Starynkevitch Dec 15 '14 at 12:54
  • See [vfork(2)](http://man7.org/linux/man-pages/man2/vfork.2.html). Notice that it is not in POSIX 2008 (and obsolete in POSIX 2001) – Basile Starynkevitch Dec 15 '14 at 13:00
  • 2
    @BasileStarynkevitch I think vfork is largely harmless and quite possibly signals intent better though is now removed from posix, however the best migration path from vfork is posix_spawn as it is faster than either and immediately execs the other program avoiding the possibility of logic errors. – Vality Dec 15 '14 at 13:02
  • Skipping file descriptors 0,1,2 solves the problem. And posix_spawn seems to be a good idea for my needs. – Lexa Dec 16 '14 at 11:40
  • 1
    Seems like you were on some kind of mission, Basile. vfork is not "useless" if you are on a system that lets vfork share address space and are not allowed to overcommit virtual memory. And exclusively manipulating file descriptors in a vforked process is safe (for example to set up a pipe from the output), as those are copied. With that said, OP's example code does have some huge problems with vfork as it dramatically messes with the stack in the child. – codetaku Jan 19 '17 at 20:31
  • This in case the execve-d program needs to open a few files. But a well behaved program should close all the useless fd-s anyway (so the loop on `close` should be useless) – Basile Starynkevitch Oct 07 '17 at 16:53
4

First problem: There is no way to prevent inheritance of file descriptors except you close them yourself or set FD_CLOEXEC, check this

Second problem: You got The return value of waitpid is often 0, because you sepecfied WNOHANG in waitpid.

waitpid(): on success, returns the process ID of the child whose state has changed; 
if WNOHANG was specified  and  one  or  more  child(ren) specified by pid exist, 
but have not yet changed state, then 0 is returned.  On error, -1 is returned.
Community
  • 1
  • 1
D3Hunter
  • 1,329
  • 10
  • 21
  • My application should work like a start panel for other programs, so I need to specify WNOHANG, otherwise - as I know - I could not call more than one program at one time. I only want to get the information if exec fails to figure out why it failed and to display a message. The problem is, that waitpit doesn't reliable returns -1 for failure. – Lexa Dec 16 '14 at 11:11
  • 1
    I think you better set a handler for `SIGCHLD`, when one program failed on exec, you'll get signal `SIGCHLD`. That's much more robust than check it with `WNOHANG` using `waitpid`. – D3Hunter Dec 16 '14 at 12:29