-2

I'm writing a program, that reads a file, and for each line, it creates a thread. In log, I can see that before calling the pthread_create command, it is taking the lines correctly (one at a time). The problem is that when creating the thread, it is randomly repeating the lines. I originally thought that a usleep(250) could solve it (in fact it mitigates the problem a bit), but it still keeps repeating some lines and ignoring others.

Please excuse the terrible mix of C with C++! The code:

#include <iostream>
#include <cstdlib>
#include <pthread.h>
#include <fstream>
#include <stdlib.h>

using namespace std;

#define NUM_THREADS     201

struct thread_data{
int  thread_id;
char *serial;
char *location;
};

void *RunCommand(void *threadarg)
{
struct thread_data *my_data;

my_data = (struct thread_data *) threadarg;

char *writecmd1="nohup date --date='10 hour' '+%Y-%m-%d_%H:%M:%S' >> /results_path/";
char *writecmda="cd /work_place/bin; ./process1 -d";
char *serial_num=my_data->serial;
char *d_location=my_data->location;

char *writecmd2=" >> /results_path/";

char *writecmd3=".out ";
char *writecmd3a="; ";
char *writecmd4="& ";

char * final_cmd = (char *) malloc(1 + strlen(writecmd1) + strlen(serial_num) + strlen(writecmd3) + strlen(writecmd3a) + strlen(writecmda) + strlen(d_location) + strlen(writecmd2) + strlen(serial_num) + strlen(writecmd3) + strlen(writecmd4));

strcpy(final_cmd,writecmd1);
strcat(final_cmd,serial_num);
strcat(final_cmd,writecmd3);
strcat(final_cmd,writecmd3a);

strcat(final_cmd,writecmda);
strcat(final_cmd,d_location);
strcat(final_cmd,writecmd2);
strcat(final_cmd,serial_num);
strcat(final_cmd,writecmd3);
strcat(final_cmd,writecmd4);

cout << final_cmd << endl;

cout << my_data->serial << "-" << my_data->location << endl;

system(final_cmd);

free(final_cmd);

pthread_exit(NULL);
}


int main ()
{
pthread_t threads[NUM_THREADS];
struct thread_data td[NUM_THREADS];
int rc;
int i = 0;
const char s[2] = ",";
string input_line;

std::ifstream infile("elements_list.in");

while (infile >> input_line)
{
char *main_line = &input_line[0u];
char *token;

// Get the first token. In the file it is the serial number
token = strtok(main_line, s);
td[i].serial = token;
// Get the second token. In the file it is the location
token = strtok(NULL, s);
td[i].location = token;

td[i].thread_id = i;

cout << td[i].serial << "@" << td[i].location << endl;
usleep(250); //Sleep for miliseconds
rc = pthread_create(&threads[i], NULL, RunCommand, (void *)&td[i]);

if (rc)
{
cout << "Error:unable to create thread," << rc << endl;
exit(-1);
}
i++;
}

pthread_exit(NULL);
}

An example of my output (the line with @ is before creating the thread, the line with hyphen is when creating the thread):

10000230@location1
10000294@location2
10000294-location2
10000294-location2
10000301@location3
10000301-location3
10000257@location4
10000257-location4
10000344@location5
10000071@location6
10000354@location7
10000354-location7
10000041@location8
10000041-location8
10000058@location9
10000058-location9
1000036310000363-location10
10000363@location10
-location10
10000363-location10
10000201@location11
10000201-location11
10000095@location12
20000037-location13
20000037@location13

On top of that in some cases my structure is apparently loosing shape (see the line for "location10"). Location 1, 5 and 6 are ignored, Location 2 repeated, etc.

The only post I found close to my issue is Multithreading in C but honestly I'm not entirely clear about the solution.

Hope you can help. Thanks in advance

Community
  • 1
  • 1
sakhmet77
  • 1
  • 1
  • 1
    What's with all the `char*`? You *do* know about [`std::string`](http://en.cppreference.com/w/cpp/string/basic_string)? – Some programmer dude Sep 29 '16 at 07:57
  • You're storing pointers into `input_line` for later and these become invalid. Give up on the C-style string management and embrace C++. – molbdnilo Sep 29 '16 at 08:13
  • Thank you. But besides replacing char* with std::string, do you have any idea why the random repetition of lines? Such repetition is completely independant of the string management. Please ignore all the crazy concatenation. If I just print the content of the structure in void *RunCommand(void *threadarg), I still see such random behaviour. what am I missing regarding the structure management? – sakhmet77 Sep 30 '16 at 10:54
  • Thank you mobdnilo, I replaced all char* with std::string and I'm getting no random repetitions. Cheers! – sakhmet77 Oct 04 '16 at 06:26

1 Answers1

0

First of all: Your program does not compile using g++. Your code example misses

#include <string.h> // or rather <cstring>
#include <unistd.h>

I have to admit that it was kind of painful to read your code. I did not find the exact reason why lines are repeated but your malformed output is due to a race condition.

std::cout::operator<< is thread safe. 2 concurrent calls are never interleaved in your output. So

Thread 1:
    std::cout << "foo\n"
Thread 2:
    std::cout << "bar\n"

Always yields either foo\nbar\n or bar\nfoo\n and never fboo\nar\n or something similar. However you are using multiple calls to std::cout::operator<<

Thread 1:
    std::cout << "f" << "o" << "o" << std::endl;
Thread 2:
    std::cout << "b" << "a" << "r" << std::endl;

Which is the same as

Thread 1:
    std::cout << "f";
    std::cout << "o";
/* ... */

may yield fboaor\n\n.

When you are multithreading you have to create your full output lines before feeding them to std::cout:

std::stringstream ss;
ss << "My" << "safe" << "output" << "scheme" << std::endl;
std::cout << ss.str();

I also strongly urge you to read a decent book on C++. Your program has a lot of other problems.

Richard
  • 1,117
  • 11
  • 31
  • Thank you Richard, truly appreciate the comments. I admit it is my first code ever, so yes, it is definitely messy. But besides the char * replacement with std, what are your observations regarding the struct management? For some reason it keeps repeating lines randomly when in void *RunCommand(void *threadarg) – sakhmet77 Sep 30 '16 at 10:56
  • It is most likely a memory corruption issue with your `char*`s as molbdnilo pointed out. Switch to `std::string` and the problem may disappear. – Richard Sep 30 '16 at 12:09
  • Fantastic Richard, many thanks. I got rid of all char* and it worked. – sakhmet77 Oct 04 '16 at 06:23