0

This program counts the occurrence of a word in a line. It runs as expected, but I have 2 concerns:

  1. delete tmp is commented as of now (line 57). If it is uncommented and compiled, my executable gives "segmentation fault". Oddly, it doesn't crash while running in gdb nor with valgrind.
  2. Lines 65 and 66: ideally these threads would need to be joined. But, I am getting correct output even though they are not joined. Is this how it behaves for a shared (volatile) variable?
#include <iostream>
#include <stdio.h>
#include <string.h>
#include <pthread.h>
#include <unistd.h>
#include <fstream>
#include <new>
#define MAX_BUFF 1000

using namespace std;
volatile int tcount=0;
pthread_mutex_t myMux;

typedef struct data
{
    string line;
    string arg;
}tdata;

void *calcWordCount(void *arg)
{
    tdata *tmp = (tdata *)arg;
    string line = tmp->line;
    string s = tmp->arg;
    int startpos = 0;
    int finds = 0;
    while ((startpos = line.find(s, startpos)) != std::string::npos)
    {
            ++finds;
            startpos+=1;
            pthread_mutex_lock(&myMux);
            tcount++;
            pthread_mutex_unlock(&myMux);
    }
    //cout<<endl<<line<<s<<" "<<finds<<endl;
}
int main(int argc,char *argv[])
{
  pthread_t thread_ids[10000];
  int cnt=1;
  int thread_cnt=0;
  void *exit_status;
  int targc=argc;
  ifstream infile("testfile");  
  string line;
  while(getline(infile,line))
  {
        while(targc >1)
        {
              tdata *tmp = new tdata;
          tmp->line = line;
              tmp->arg = argv[cnt];
              pthread_create(&thread_ids[thread_cnt],NULL,calcWordCount,tmp);
              thread_cnt++;
              cnt++;
              targc--;
              //delete tmp;
        }
    cnt=1;
    targc=argc;

}
infile.close();
int j;
/*for(j=0;j<thread_cnt;j++)
    pthread_join(thread_ids[j],&exit_status);*/
cout<<tcount<<endl;
return 0;
} 
9
  • You can't delete tmp because the thread you just created is still running.
    – Paul R
    Jun 9, 2014 at 8:54
  • 2
    You'll need to keep the tmp data pointers until you have joined all of your threads. Jun 9, 2014 at 8:54
  • 1
    @Ajit 'how will I keep track' You may use a std::vector<tdata*> for instance. Jun 9, 2014 at 9:00
  • 1
    Also keep in mind that volatile is not sufficient to make tcount thread safe. Consider using std::atomic<int> tcount instead.
    – nwp
    Jun 9, 2014 at 9:06
  • 1
    @Ajit The volatile is useless in your example. As a rule of thumb never use volatile in combination with threads. It will break your code in very interesting ways. Also there is std::thread and std::async to make your life much easier.
    – nwp
    Jun 9, 2014 at 10:04

1 Answer 1

0

You have undefined behaviour as you're calling delete on tmp and another thread is consuming it.

One fix would be to create and pass (by value) a std::shared_ptr rather than the bare pointer into your worker thread. You can use release and reset methods in your main loop. The memory will be released by std::shared_ptr once the worker thread is done with it.

2
  • Thanks for your suggestion. I have never used shared_ptr, let me just goole how to use them. It it works I will update here.. :) Jun 9, 2014 at 9:04
  • Hey, I am unable to find appropriate example of shared_ptr with threads. Can you please put some light on this. Jun 9, 2014 at 10:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.