How to store process IDs and kill them?

Question

I'm having trouble with dealing with background processes in my shell. I'm storing the pids in an array and then when I call 'jobs', it's supposed to iterate through the array and if the number is not 0 and if it is still alive, it prints, if not, it's supposed to remove the pid. I'm getting very inconsistent results. Can someone see what I'm doing wrong? Thanks Result:

ben$ sleep 5 &
Ben: started job 30290
ben$ jobs
30290
ben$ jobs
30290
ben$ jobs
30290
ben$ kill 30290
ben$ kill 30290: No such process
jobs
ben$ jobs
ben$ sleep 4s &
Ben: started job 30547
ben$ jobs
30547
ben$ kill 30547
ben$ jobs
30547
ben$ jobs
30547
ben$ 

First file is main, and the second one is my functions file

/*
  Shell 1
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <sys/wait.h>
#include <stdbool.h>
#define DELIMS " \t\r\n"
int main (int argc, char *argv[]) {
  int ids[10];

  initializeArray(ids);

  while (1) {
      char line[100];
      char *temp;
      char *split[15];
      pid_t pid;
      bool background=false;

    printf("ben$ ");

    //if cmd-d, exit shell
   if(!fgets(line, sizeof(line), stdin)) {
      printf("\n");
      break;
   } 

      line[strlen(line)-1]='\0';
      temp=strtok(line," ");


      if (strcmp(temp, "exit") == 0) {
        break;
      }
      else if (strcmp(temp, "cd") == 0) {
        char *arg = strtok(0, DELIMS);

        if (!arg) {
          fprintf(stderr, "cd missing argument.\n");
        }
        else {
          chdir(arg);
        }

      } 
      else if (strcmp(temp, "jobs") == 0) {
        printJobs(ids);
        continue;
      }

      else {
      int i=0;
      while(temp != NULL) {
        split[i]=temp;
        temp=strtok(NULL," ");
        i++;
      }
      split[i] = NULL;
      if(strcmp(split[i-1], "&")==0) {
        // printf("should do in background");
        split[i-1]='\0';
        background=true;
      }

      char *args[i];
      int j;
      for(j=0; j < i; j++){
        args[j]=split[j];
      }


      pid=fork();

      if(pid==0) {
        if(background) {
                  fclose(stdin);
                  fopen("/dev/null","r");
        }
        execvp(args[0], args);
        printf("This should not happen\n");
      }
      else {
        if(!background) {
           // printf("Not in background\n");
           wait(&pid);
        }
        else {
           printf("Ben: started job %d\n",pid);
           insertPid(ids,pid);
           background=false;
        }
     }
    }

  }
  return 0;
}


#include <stdio.h>
#include <stdlib.h>
#include <string.h>


void initializeArray(int ids[]);
void insertPid(int ids[],int pid);
void removePid(int ids[],int pid);
void printJobs(int ids[]);


void initializeArray(int ids[]) {
    int i;
    for(i=0;i<10;i++) {
        ids[i]=0;
    }

}

void insertPid(int ids[],int pid) {
    // printf("inserting\n");
    int i=0;


    while(i<10) {
        if(ids[i]==0) {
            ids[i]=pid;
            return;
        }
        i++;
    }
    printf("insert into list error");
    return;
}
void removePid(int ids[],int index) {
    // printf("removing %d\n",pid );
       ids[index]=0;
        return;
}
void printJobs(int ids[]) {
    // printf("printing\n");
    int i;
    int pid;
    for(i=0;i<10;i++) {

        if(ids[i] != 0) {
            // printf("job value %d",ids[i]);
            pid=ids[i];
            if(kill(pid,0) == 0) {
                printf("%d\n",pid);
            }
            else{
                removePid(ids,i);
            }
        }
    }
}

Answer 1

How do you know if your (background) jobs have died? You wait on your foreground job - so you know when they have completed - but you don't handle the background jobs. Whether you kill the children or they eventually expire on their own (e.g. sleep runs out in the example case) you won't know it. That is what you are seeing in your output listing too.

One way to approach this is to set up a signal handler for SIGCHLD and waitpid on them when they die. Then update your jobs array. Or some variation thereof.