Doing my own shell, how to properly execute processes in background/foreground?

Also check out popen() -- unless you are supposed to use exec(), of course.

If you use system() inside the fork, you can wait for the process to return. Because it's forked, it will still be a "background" process; the parent is not held up by the child (and the child is not doing anything else anyway). It is not as if exec were an advanced feature and system a simple one provided for beginners ;) They do serve slightly different purposes.

You already suggested all that and I already explained why I can't go that way.

Anyway, I might just use a global variable to fix my first problem. I've always been a little paranoid about global variables but I think that in this case it's no harm... I'll wait for my teacher's e-mail reply though.

Thanks to everyone so far. A friend gave me some hints and I'll see what I can do with them, I'll post back of I have any more questions...

Instead of using a global variable I did something like this instead:

Code:

---

if(!background) {
        signal(SIGCHLD, NULL);
       
        waitpid(pid, NULL, 0);
       
        signal(SIGCHLD, childSignalHandler);
}

---

If I'm running a foreground process "delete" the handler for SIGCHLD so it doesn't get called. Then, after waitpid(), set the handler again. This way, only the background processes will be handled.

Do you think there's anything wrong with this solution?

Quote:

---

Originally Posted by Nazgulled

Instead of using a global variable I did something like this instead:

Code:

---

if(!background) {
        signal(SIGCHLD, NULL);
       
        waitpid(pid, NULL, 0);
       
        signal(SIGCHLD, childSignalHandler);
}

---

If I'm running a foreground process "delete" the handler for SIGCHLD so it doesn't get called. Then, after waitpid(), set the handler again. This way, only the background processes will be handled.

Do you think there's anything wrong with this solution?

---

Yes. The second argument to signal() can't be NULL. You can pass SIG_IGN as the function, meaning "ignore". But then you have the problem that any SIGCHLD delivered while you're waiting is gone forever. Use sigprocmask() to block instead. The signal will be delivered when you stop blocking.

In answer to your earlier question about how to get rid of zombies without a signal handler, you can just repeatedly call waitpid(-1, &status, WNOHANG) until it returns -1. Do that each iteration of your main loop and you'll reap the zombies. The downside to this method, though, is that the zombies will stick around until the user gives some input.

Quote:

---

Originally Posted by cas

Yes. The second argument to signal() can't be NULL. You can pass SIG_IGN as the function, meaning "ignore". But then you have the problem that any SIGCHLD delivered while you're waiting is gone forever.

---

Also, someone else said to me that I might have race condition problems with that code so I just removed it all together...

Quote:

---

Originally Posted by cas

Use sigprocmask() to block instead. The signal will be delivered when you stop blocking.

---

But where exactly should I start and stop blocking?

Quote:

---

Originally Posted by cas

In answer to your earlier question about how to get rid of zombies without a signal handler, you can just repeatedly call waitpid(-1, &status, WNOHANG) until it returns -1. Do that each iteration of your main loop and you'll reap the zombies. The downside to this method, though, is that the zombies will stick around until the user gives some input.

---

I think you misinterpreted me, I never said I wanted to get rid of zombies without a signal handler, I need it with a signal handler. But that's done already...

I've fixed my problem with something teacher suggested me but this introduced another problem that I can't seem to fix. And he haven't yet answered me back...

The problem now, is that with this new code, if a command doesn't exist, my bash will stall in the while(fgWait) pause(); loop. It will stall there because as the command doesn't exist, execvp() will return an error and the child process will be immediately exited with exit status = 1 and so, when the parent process reaches the while(fgWait) loop, it will be waiting for a signal that was already sent.

Here's my updated code:

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <wait.h>
#include <signal.h>
#include <sys/types.h>
#include <errno.h>
#include <readline/readline.h>
#include <readline/history.h>

#include "data.h"


volatile bool fgWait = FALSE;
pid_t fgPid = -1;


void childSignalHandler(int signum) {
        int status;
        pid_t pid;
       
        #ifdef WCONTINUED
                pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED | WCONTINUED);
        #elif
                pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED);
        #endif
       
        if(pid == -1) {
                perror("waitpid");
                exit(1);
        }

        if(pid > 0) {
                if(fgPid == pid) {
                        fgPid = -1;

                        fgWait = FALSE;
                } else {
                        if(WIFEXITED(status)) {
                                printf("[%d] Terminated (Status: %d)\n", pid,
                                        WEXITSTATUS(status));
                                return;
                        }
                }

                if(WIFSIGNALED(status)) {
                        printf("[%d] Terminated (Signal: %d)\n", pid, WTERMSIG(status));
                        return;
                }

                if(WIFSTOPPED(status)) {
                        printf("[%d] Stopped (Signal: %d)\n", pid, WSTOPSIG(status));
                        return;
                }

                #ifdef WIFCONTINUED
                        if(WIFCONTINUED(status)) {
                                printf("[%d] Continued...\n", pid);
                                return;
                        }
                #endif
        }
}

void intstpSignalHandler(int signum) {
        if(fgPid != -1) {
                kill(fgPid, signum);
        }
}

int main(int argc, char **argv) {
        char *bBuffer = NULL, *pArgs[MAXARGNUM], *aPtr = NULL, *sPtr;
        bool bgProcess;
        int aCount;
        pid_t pid;

        signal(SIGINT, intstpSignalHandler);
        signal(SIGTSTP, intstpSignalHandler);
       
        signal(SIGCHLD, childSignalHandler);

        using_history();

        while(1) {
                free(bBuffer);
               
                bBuffer = readline("\e[1;31mraBash \e[1;32m# \e[0m");
               
                add_history(bBuffer);
               
                if(!strcasecmp(bBuffer, "exit")) {
                        exit(0);
                }
               
                sPtr = bBuffer;
                aCount = 0;
               
                do {
                        aPtr = strsep(&sPtr, " ");
                        pArgs[aCount++] = aPtr;
                } while(aPtr);

                bgProcess = FALSE;
               
                if(!strcmp(pArgs[aCount-2], "&")) {
                        pArgs[aCount-2] = NULL;
                        bgProcess = TRUE;
                }
               
                if(strlen(pArgs[0]) > 1) {
                        pid = fork();
                       
                        if(pid == -1) {
                                perror("fork");
                                exit(1);
                        }
                       
                        if(pid == 0) {
                                if(execvp(pArgs[0], pArgs) == -1) {
                                        if(errno == ENOENT) {
                                                printf("myBash: %s: command not found\n", pArgs[0]);
                                        }
                                       
                                        exit(1);
                                }
                        } else {
                                if(!bgProcess) {
                                        fgPid = pid;
                                       
                                        fgWait = TRUE;

                                        while(fgWait) {
                                                pause();
                                        }
                                } else {
                                        printf("[%d] Started...\n", pid);
                                }
                        }
                }
        }

        return 0;
}

---

Any suggestions to fix this new problem?

Should the parent be programmed to pause() or wait() for the child process to stop / terminate??

I think I solved my own problem, here's my updated code:

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <wait.h>
#include <signal.h>
#include <sys/types.h>
#include <errno.h>
#include <readline/readline.h>
#include <readline/history.h>

#include "data.h"


volatile bool fgWait = FALSE;
volatile bool sigRace = TRUE;
pid_t fgPid = -1;


void childSignalHandler(int signum) {
        int status;
        pid_t pid;
       
        #ifdef WCONTINUED
                pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED | WCONTINUED);
        #elif
                pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED);
        #endif
       
        if(pid == -1) {
                perror("waitpid");
                exit(1);
        }
       
        if(pid > 0) {
                if(fgPid == pid) {
                        fgPid = -1;

                        fgWait = FALSE;
                } else {
                        if(WIFEXITED(status) && sigRace) {
                                sigRace = FALSE;
                                return;
                        }

                        if(WIFEXITED(status)) {
                                printf("[%d] Terminated (Status: %d)\n", pid,
                                        WEXITSTATUS(status));
                                return;
                        }
                }
               
                if(WIFSIGNALED(status)) {
                        printf("[%d] Terminated (Signal: %d)\n", pid, WTERMSIG(status));
                        return;
                }
               
                if(WIFSTOPPED(status)) {
                        printf("[%d] Stopped (Signal: %d)\n", pid, WSTOPSIG(status));
                        return;
                }

                #ifdef WIFCONTINUED
                        if(WIFCONTINUED(status)) {
                                printf("[%d] Continued...\n", pid);
                                return;
                        }
                #endif
        }
}

void intstpSignalHandler(int signum) {
        if(fgPid != -1) {
                kill(fgPid, signum);
        }
}

int main(int argc, char **argv) {
        char *bBuffer = NULL, *pArgs[MAXARGNUM], *aPtr = NULL, *sPtr;
        bool bgProcess;
        int aCount;
        pid_t pid;

        signal(SIGINT, intstpSignalHandler);
        signal(SIGTSTP, intstpSignalHandler);

        signal(SIGCHLD, childSignalHandler);
       
        using_history();

        while(1) {
                free(bBuffer);
               
                bBuffer = readline("\e[1;31mraBash \e[1;32m# \e[0m");
               
                if(!strcasecmp(bBuffer, "exit")) {
                        exit(0);
                }

                if(strlen(bBuffer) > 0) {
                        add_history(bBuffer);
                }
               
                sPtr = bBuffer;
                aCount = 0;
               
                do {
                        aPtr = strsep(&sPtr, " ");
                        pArgs[aCount++] = aPtr;
                } while(aPtr);

                bgProcess = FALSE;
               
                if(!strcmp(pArgs[aCount-2], "&")) {
                        pArgs[aCount-2] = NULL;
                        bgProcess = TRUE;
                }
               
                if(strlen(pArgs[0]) > 1) {
                        sigRace = TRUE;

                        pid = fork();

                        if(pid == -1) {
                                perror("fork");
                                exit(1);
                        }

                        if(pid == 0) {
                                if(execvp(pArgs[0], pArgs) == -1) {
                                        if(errno == ENOENT) {
                                                printf("myBash: %s: command not found\n", pArgs[0]);
                                        }
                                       
                                        exit(1);
                                }
                        } else {
                                if(!bgProcess) {
                                        fgPid = pid;

                                        if(sigRace) fgWait = TRUE;
                                        else fgWait = FALSE;

                                        while(fgWait) {
                                                pause();
                                        }
                                } else {
                                        printf("[%d] Started...\n", pid);

                                        sigRace = FALSE;
                                }
                        }
                }
        }

        return 0;
}

---

If you think there's anything wrong, let me know please...

May you post your header files?

Quote:

---

Originally Posted by Nazgulled

If you think there's anything wrong, let me know please...

---

You are still calling printf() within a signal handler. You can't do that.

Not so fast brewbuck, listen to the OP

Quote:

---

Originally Posted by Nazgulled

I'm not sure what you mean because I just added a printf("CALLING WAITPID...\n"); right before pid = waitpid(-1, &status, WNOHANG); in childSignalHandler() and it's working just fine...

---

Quote:

---

Originally Posted by MK27

Not so fast brewbuck, listen to the OP

---

There are plenty of things you can do in C which seem to work but are completely wrong.

On average, you survive Russian Roulette -- wanna play?

Quote:

---

Originally Posted by Ashen

May you post your header files?

---

What do you mean? If you have installed the necessary packages on your system for building C apps, you already have them... IF you are talking about data.h, it's just 3 macros and a typedef, not really relevant for anything...

Quote:

---

Originally Posted by brewbuck

You are still calling printf() within a signal handler. You can't do that.

---

I really don't know what you mean by that... If you're saying that I can't do that because it won't work, you're wrong because it works as MK27 said by quoting me from the previous page. If you're saying I shouldn't do that despite working, I would love to know and understand why.

Bear in mind that I showed this "final" code to my teacher and he had nothing to say about it, other than letting me know that I've finished the work for that exercise.

Quote:

---

Originally Posted by MK27

Not so fast brewbuck, listen to the OP

---

Not so fast MK27, listen to the man page: ;-)

Quote:

---

Originally Posted by man 7 signal

A signal handling routine established by sigaction(2) or signal(2) must
be very careful, since processing elsewhere may be interrupted at some
arbitrary point in the execution of the program. POSIX has the concept
of "safe function". If a signal interrupts the execution of an unsafe
function, and handler calls an unsafe function, then the behavior of
the program is undefined.

POSIX.1-2004 (also known as POSIX.1-2001 Technical Corrigendum 2)
requires an implementation to guarantee that the following functions
can be safely called inside a signal handler:

*long list of functions follows, of which printf is not one.*

---

...which is what brewbuck hinted at. But I myself never found any way to notify the main thread of an event from a signal handler. (and hence, I dropped signals altogether.) It would be interesting if someone could enlighten us -- If you have a SIGCHLD handler, how do you have the main thread wait for it to fire, and get information back from it?

Quote:

---

Originally Posted by Nazgulled

I really don't know what you mean by that... If you're saying that I can't do that because it won't work, you're wrong because it works as MK27 said by quoting me from the previous page. If you're saying I shouldn't do that despite working, I would love to know and understand why.

---

Because most of the time, you are NOT inside printf() when the signal is delivered. But if you are, things have the potential to go very wrong as both printf() invokations simultaneously manipulate the internals of the stdout data structure.

I once helped a coworker track down a bug that was caused by calling malloc() inside a signal handler. The bug only showed up every few weeks. The stars have to align just right such that a call to malloc() is already in progress when the signal is delivered. The two concurrent malloc() calls completely trashed the heap.

He'd been trying to find that bug for a month. It took five minutes of looking at his signal handler to see what was wrong. He switched to a different strategy, and the bug went away.

In general, it's dangerous to do almost ANYTHING inside a signal handler except manipulate volatile sig_atomic_t variables, and make system calls.

EDIT: As far as the prof not having any comment on this wrongness, I'm not terribly surprised. You learn all kinds of wrong things in school.