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;
} 
Paul Roub
  • 36,322
  • 27
  • 84
  • 93
unbesiegbar
  • 471
  • 2
  • 7
  • 19
  • You can't delete `tmp` because the thread you just created is still running. – Paul R Jun 09 '14 at 08:54
  • 2
    You'll need to keep the `tmp` data pointers until you have joined all of your threads. – πάντα ῥεῖ Jun 09 '14 at 08:54
  • but then, how will I keep track of those to free them later ? There may be multiple such structures allocated. – unbesiegbar Jun 09 '14 at 08:57
  • 1
    @Ajit _'how will I keep track'_ You may use a `std::vector` for instance. – πάντα ῥεῖ Jun 09 '14 at 09:00
  • @ ^, Thanks for the suggestion, I will try this. Also, Bathsheba has suggested to use shared_ptr let me see if it works. – unbesiegbar Jun 09 '14 at 09:02
  • 1
    Also keep in mind that `volatile` is not sufficient to make `tcount` thread safe. Consider using `std::atomic tcount` instead. – nwp Jun 09 '14 at 09:06
  • @nwp I didn't knew it...thanks for the info. btw, I got the volatile info from this link http://comsci.liu.edu/~murali/unix/Mutex.htm – unbesiegbar Jun 09 '14 at 09:16
  • 1
    @Ajit The `volatile` is useless in your example. As a rule of thumb [never use `volatile` in combination with threads](http://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading). It will break your code in very interesting ways. Also there is [std::thread](http://www.cplusplus.com/reference/thread/thread/) and [std::async](http://www.cplusplus.com/reference/future/async/) to make your life much easier. – nwp Jun 09 '14 at 10:04
  • @ πάντα ῥεῖ , I have implemented the vector ptrs solution and using for(j=0;j – unbesiegbar Jun 09 '14 at 10:19

1 Answers1

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.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 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.. :) – unbesiegbar Jun 09 '14 at 09:04
  • Hey, I am unable to find appropriate example of shared_ptr with threads. Can you please put some light on this. – unbesiegbar Jun 09 '14 at 10:05