forked threads in C are writing each other stdout

Question

I wrote a little daemon.

This is the flow of the daemon, in general:

Get variables
Get all rows from the database (may be either MySQL or Oracle) that meets the query parameters (in this case get all rows that has this current time).
If any rows were found then run a Perl script for each row (using execv).

This daemon works well, but the problem is that when I have two rows or more coming back from the query, they start, but the Perl script outputs are mixed. They are supposed to run independently without interfering with each other.

Am I doing something wrong?

Is this a memory leak?

Here is my code :

/*
 ============================================================================
 Name        : main.c
 Description : Daemon for scheduler
 ============================================================================
 */

#include <sys/types.h>
#include <sys/wait.h>
#include <sys/stat.h>
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <unistd.h>
#include <syslog.h>
#include <string.h>
#include <strings.h>
#include <regex.h>
#include <time.h>

#ifdef MYSQL_CODE
#include <mysql.h>
#endif

#ifdef ORACLE_CODE
#include <ocilib.h>
#endif

#define DAEMON_NAME "schedulerd"

void start_job(char *automation_user,char *automation_path,char *automation_log_path, char *id, char *site, char *db_type,char *db_host,char *db_user,char *db_password,char *db_schemata){

    pid_t pid;
    pid = fork();

    if (pid < 0) { exit(EXIT_FAILURE); }

    //We got a good pid, Continue to the next result
    if (pid > 0) {
        return;
    }

    char *file_format = "scheduler";
    char *seperator = "_";
    char *postfix = "_XXXXXX";
    char *extension = ".log";

    //get Time
    time_t now;
    struct tm *now_tm;
    char hour[2],min[2],sec[2];

    now = time(NULL);
    now_tm = localtime(&now);
    sprintf(hour, "%d", now_tm->tm_hour);
    sprintf(min, "%d", now_tm->tm_min);
    sprintf(sec, "%d", now_tm->tm_sec);

    char *str  = (char *)automation_log_path;
    strcat(str,(char *)file_format);
    strcat(str,seperator);
    strcat(str,hour);
    strcat(str,seperator);
    strcat(str,min);
    strcat(str,seperator);
    strcat(str,sec);
    strcat(str,postfix);

    // buffer to hold the temporary file name
    char nameBuff[128];

    int filedes = -1;

    // memset the buffers to 0
    memset(nameBuff,0,sizeof(nameBuff));

    // Copy the relevant information in the buffers
    strncpy(nameBuff,str,128);

    // Create the temporary file, this function will replace the 'X's
    filedes = mkstemp(nameBuff);

    if(filedes<1)
    {
        syslog (LOG_NOTICE, "Creation of temp file [%s] failed with error [%s]\n",nameBuff,strerror(errno));
    }else{
        mode_t perm = 0644;
        fchmod(filedes, perm); //Change permission to the file so everyone can read

        close(filedes); // Close created file
        //Rename file
        int ret;
        char newname[128];
        sprintf(newname, "%s%s", nameBuff,extension);
        ret = rename(nameBuff, newname);
        if(ret != 0) {
            syslog (LOG_NOTICE, "Renaming  of temp file %s to %s failed \n",nameBuff,newname);
            exit(EXIT_FAILURE);
        }

        char statement[256];
        sprintf(statement, "UPDATE scheduler_list SET log_file='%s' WHERE id='%s'", newname,id);
        syslog (LOG_NOTICE,"Adding to DB : %s\n", statement);

        char command[2048];
        sprintf(command, "cd %s ; ./runner.pl -site %s -log %s -scheduler_id %s -command \\\"./run_me.pl\\\"", automation_path,site,newname,id);
        //sprintf(command, "cd /net/10.7.5.50/opt/trunk/ ; ./runner.pl -site %s -log %s -scheduler_id %s -command \\\"./run_me.pl\\\"",site,newname,id);

        if (strcasestr(db_type,"mysql")){/* mysql */
#ifdef MYSQL_CODE
            MYSQL *conn;
            //mysql_free_result(res); // Free mysql
            conn = mysql_init(NULL);
            /* Connect to database */
            if (!mysql_real_connect(conn, db_host,db_user, db_password, db_schemata, 0, NULL, 0)) {
                syslog (LOG_NOTICE,"%s\n", mysql_error(conn));
                exit(EXIT_FAILURE);
            }

            if (mysql_query(conn,statement)) {
                syslog (LOG_NOTICE,"%s\n", mysql_error(conn));
                exit(EXIT_FAILURE);
            }
#endif
        }else{
#ifdef ORACLE_CODE
            OCI_Connection* cn;
            OCI_Statement* st;
            OCI_Initialize(NULL, NULL, OCI_ENV_DEFAULT);
            char query_command[128];
            sprintf(query_command, "%s:1521/%s", db_host,db_schemata);
            cn = OCI_ConnectionCreate(query_command, db_user, db_password, OCI_SESSION_DEFAULT);
            st = OCI_StatementCreate(cn);
            OCI_Prepare(st, statement);
            OCI_Execute(st);
            OCI_Commit(cn);
            OCI_Cleanup();
#endif
        }

        char *args[] = {"sudo", "-u", automation_user, "bash","-c",command,NULL};

        FILE *log_file_h = fopen(newname, "w");
        if (log_file_h == NULL)
        {
            syslog (LOG_NOTICE,"Error opening file %s !\n",newname);
            exit(EXIT_FAILURE);
        }

        syslog (LOG_NOTICE,"Starting scheduler job %s , command : %s",id, command);
        fclose(log_file_h);

        execv("/usr/bin/sudo",args);
        syslog (LOG_NOTICE,"Failed to start job %s ",id);
        perror("error");
    }
    exit(EXIT_FAILURE);
}

void process(char *automation_user,char *automation_path, char *automation_log_path ,char *db_type,char *db_host,char *db_user,char *db_password,char *db_schemata){

    if (strcasestr(db_type,"mysql")){/* mysql */
#ifdef MYSQL_CODE
        MYSQL *conn;
        MYSQL_RES *res;
        MYSQL_ROW row;
        conn = mysql_init(NULL);
        /* Connect to database */
        if (!mysql_real_connect(conn, db_host,db_user, db_password, db_schemata, 0, NULL, 0)) {
            syslog (LOG_NOTICE,"%s\n", mysql_error(conn));
            return;
        }
        /* send SQL query */
        if (mysql_query(conn, "SELECT id,site from scheduler_list where start_date = DATE_FORMAT(now(),'%Y-%m-%d %k:%i:00') AND id != father_id AND run='yes'")) {
            syslog (LOG_NOTICE,"%s\n", mysql_error(conn));
            return;
        }
        res = mysql_use_result(conn);
        /* output table name */
        while ((row = mysql_fetch_row(res)) != NULL){
            char *id = malloc(strlen(row[0]) +1);
            strcpy(id,row[0]);
            char *site = malloc(strlen(row[1]) +1);
            strcpy(site,row[1]);

            start_job(automation_user,automation_path,automation_log_path,id,site,db_type, db_host,db_user,db_password,db_schemata);
        }
        /* close connection */
        mysql_free_result(res);
        mysql_close(conn);
#endif
    }else{/* oracle */
#ifdef ORACLE_CODE
        OCI_Connection* cn;
        OCI_Statement* st;
        OCI_Resultset* rs;
        OCI_Initialize(NULL, NULL, OCI_ENV_DEFAULT);
        char query_command[128];
        sprintf(query_command, "%s:1521/%s", db_host,db_schemata);
        cn = OCI_ConnectionCreate(query_command, db_user, db_password, OCI_SESSION_DEFAULT);
        st = OCI_StatementCreate(cn);

        OCI_ExecuteStmt(st, "SELECT id,site from scheduler_list where to_char(start_date, 'yyyy-mm-dd hh24:mi')  =  to_char(SYSDATE, 'yyyy-mm-dd hh24:mi')  AND id != father_id AND run='yes'");

        rs = OCI_GetResultset(st);
        while (OCI_FetchNext(rs)){

            char *id = malloc(strlen(OCI_GetString(rs, 1)) +1);
            strcpy(id,OCI_GetString(rs,1));
            char *site = malloc(strlen(OCI_GetString(rs,2)) +1);
            strcpy(site,OCI_GetString(rs,2));
            start_job(automation_user,automation_path,automation_log_path,id,site,db_type, db_host,db_user,db_password,db_schemata);
        }
        OCI_Cleanup();
#endif
    }


}

char * set_conf_param (char *line, int addSlash){
    char *param =  malloc(strlen(line) + 2 + addSlash);
    strcpy(param,line);
    param = strchr(line,'=');
    param = param+1; //Remove '='
    strtok(param, "\n"); //remove /n
    if (addSlash == 1){
        int len = strlen(param);
        param[len] = '/';
        param[len+1] = '\0';
    }
    return strdup(param);
}

int main(int argc, char *argv[]) {
        FILE * fp;
        char * line = NULL;
        size_t len = 0;
        int found_db = 0;
        ssize_t read;
        pid_t pid, sid;
        char *automation_user=NULL,*automation_log_path=NULL ,*db_type=NULL, *db_host=NULL , *db_user=NULL, *db_password=NULL, *db_schemata=NULL;
        char *automation_path = getenv("AUTOMATION_PATH");
        //char *automation_path = "/net/10.7.5.50/opt/trunk/";

        char *automation_user_search = "automation_user=";
        char *automation_log_path_search = "automation_log=";
        char *db_type_search = "type=";
        char *db_host_search = "host=";
        char *db_user_search = "user=";
        char *db_password_search = "password=";
        char *db_schemata_search = "schemata=";
        const char comment = '#';
        /* Change the working directory to the root directory */
        /* or another appropriated directory */
        chdir(automation_path);

        //Set our Logging Mask and open the Log
        setlogmask(LOG_UPTO(LOG_NOTICE));
        openlog(DAEMON_NAME, LOG_CONS | LOG_NDELAY | LOG_PERROR | LOG_PID, LOG_USER);

        syslog(LOG_NOTICE, "Entering Daemon");

        //Read framework.conf
        fp = fopen("framework.conf", "r");
        if (fp == NULL){
            syslog (LOG_NOTICE,"Failed to open framework.conf");
            exit(1);
        }

        //Read framework.conf
        fp = fopen("framework.conf", "r");
        if (fp == NULL){
            syslog (LOG_NOTICE,"Failed to open framework.conf");
            exit(1);
        }
        while ((read = getline(&line, &len, fp)) != -1) {

            //If line commented
            if (strchr(line,comment) != NULL){
                continue;
            }

            if (strstr(line,automation_user_search) != NULL){
                automation_user = set_conf_param(line,0);
            }
            else if (strstr(line,automation_log_path_search) != NULL){
                automation_log_path = set_conf_param(line,1);
            }

            else if (db_type!=NULL && strcasestr(line,db_type) != NULL){
                found_db = 1;
            }
            else if (strstr(line,db_type_search) != NULL){
                db_type = set_conf_param(line,0);
            }
            else if (found_db && db_host==NULL && strstr(line,db_host_search) != NULL){
                db_host = set_conf_param(line,0);
            }
            else if (found_db && db_user==NULL && strstr(line,db_user_search) != NULL){
                db_user = set_conf_param(line,0);
            }
            else if (found_db && db_password==NULL && strstr(line,db_password_search) != NULL){
                db_password = set_conf_param(line,0);
            }
            else if (found_db && db_schemata==NULL && strstr(line,db_schemata_search) != NULL){
                db_schemata = set_conf_param(line,0);
            }
        }

        fclose(fp);
        if (line)
            free(line);

        if (automation_user==NULL){
            automation_user = "root";
        }

        //Fork the Parent Process
        pid = fork();

        if (pid < 0) { exit(EXIT_FAILURE); }

        //We got a good pid, Close the Parent Process
        if (pid > 0) { exit(EXIT_SUCCESS); }

        //Change File Mask
        umask(0);

        //Create a new Signature Id for our child
        sid = setsid();
        if (sid < 0) { exit(EXIT_FAILURE); }

        //Close Standard File Descriptors
        close(STDIN_FILENO);
        close(STDOUT_FILENO);
        close(STDERR_FILENO);

        //----------------
        //Main Process
        //----------------
        while(1){
            process(automation_user,automation_path,automation_log_path,db_type,db_host,db_user,db_password,db_schemata);    //Run our Process
            sleep(60);    //Sleep for 60 seconds
        }

        //Close the log
        closelog ();

        exit(EXIT_FAILURE);
}

Have You tried `valgrind` or other tools for tracing down memory leaks? — Kamiccolo, Jun 13 '16 at 12:30
I'm not clear how you expect the output from forked processes that run in parallel to be segregated. If all the output is going to one device then you need to write an explicit locking mechanism that prevents the `print` statements from being interleaved. I don't think your idea of a memory leak is relevant. — Borodin, Jun 13 '16 at 12:32
I can't see where Perl logs from forked processes (_not_ threads) wind up. They appear to write to a buffer of sorts (in-memory file?) which you rename to `newname`, but I don't see how that would go to those declared temporary file(s). Do they end up on `STDOUT,` sort of accidentally, which is then all dumped to one place? _Where_ are they "_mixed_"? It appears that they are intended to go to files with different names. Of course, these are just guesses since we aren't told where the scripts write, and this is far too much code to wade around to figure it out. — zdim, Jun 14 '16 at 03:28

score 7 · Answer 1 · answered Jun 13 '16 at 12:36

You don't appear to have made any effort to write a minimal, complete verifiable example.

This is just a dump of your whole program, including dependencies on a config file you don't provide, and databases whose schemata you don't show. None of which are even relevant to your question.

At least you edited out the company name, but for reference it's still visible in the edit history.
threads in C are writing each other stdout

I can't see any threads in your program, and you don't fork threads anyway - what you have are child processes
... are writing each other stdout

well, stdout is inherited from the parent, so all children (and their sudo/bash/perl/whatever children) are writing to the same place. The writes themselves are locked, but can be interleaved.

If you want their output not to be interleaved, have them write to different places, and figure out how to display/combine them later.

Per-child-process temporary files are a popular choice, but note the parent will have to track child process completion in order to know when to print and then delete each file. Also, you should probably detach child processes from the terminal, close stdin, and consider what to do with stderr.
Is this a memory leak?

No.

yes , you are right , I just dumped my code and didn't follow the rules , my mistake. thanks for reviewing it thought. This is my first time daemon I wrote , and C is not my main language. So here are my questions - how can I detach the child processes ? I didn't figure that out.. all other comments are well understood — Mojo, Jun 13 '16 at 12:42
Look up how to create a daemon. The distinction you can't do the second fork if the parent needs to track the child (via the pid_t returned from `fork`, or by `waitpid`). You'll need to add some mechanism for the child and parent to communicate directly if you want that 2nd fork, eg. with a pipe or socketpair, or with a temp file in a known location etc. — Useless, Jun 13 '16 at 12:51
@Mojo You can find a list of steps here: http://stackoverflow.com/a/17955149/646887 — Klas Lindbäck, Jun 13 '16 at 13:09

score -1 · Answer 2 · answered Jun 13 '16 at 12:42

I guess (after a quick glimpse) that the problem is in

execv("/usr/bin/sudo",args);

you should make sure that only one instance is running. this can be done by mutual exclusion. A good description is on: http://www.thegeekstuff.com/2012/05/c-mutex-examples/

In short:

// create a global mutex variable
pthread_mutex_t lock;

// initialise it in main
pthread_mutex_init(&lock, NULL);

// enclose your critical path
pthread_mutex_lock(&lock);
execv("/usr/bin/sudo",args);
pthread_mutex_unlock(&lock);

Hope it Helps

execv only returns on failure. This means you will never release the lock on a successful execv — ljden, Mar 31 '21 at 09:58

forked threads in C are writing each other stdout

2 Answers2