1

First of all sorry for posting a long question, but please do keep patience :).

As a part of my application i am using threads to read different parts of a memory mapped file, as in first half of the mapped file is read and processed by main thread and the second half of the mapped file is read and processed by another thread.

I am using a condition variable, to make sure that the second thread waits until the main thread populates the file related values in a structure.

Using this methodology I get the file size populated correctly sometimes and sometimes its not showing correctly.

Here is the applicable code :

void * thread_proc(void * arg){

    struct thread_related_data * dat_str = (struct thread_related_data *) arg;

    //pthread_detach(pthread_self());

    pthread_mutex_lock(&(dat_str->mutex));

    while(dat_str->thread_indicator == 0 ){
        pthread_cond_wait(&(dat_str->cond),&(dat_str->mutex));
    }

    if(dat_str->thread_indicator == 1){   // Start data processing

// start processing bottom half of the mapped file and insert data into &dat_str->thread_proc_data

        pthread_mutex_unlock(&(dat_str->mutex));
        printf ("Got conditioned\n");

        int bytes = dat_str->file_size - dat_str->start_bytes; // bytes should have almost half of the length of the file in bytes, but it does not always print the value correctly in the next statement .

        printf(" Bytes :: %d\n", dat_str->start_bytes); // Sometimes this line gets printed twice ..dont know how !!!

        char *start;
        int i = 0;
        while(i <= bytes){
            start = dat_str->file;
            while(*(dat_str->file) != '\n'){
                //printf file++;
            }

            line_proc(start,static_cast(dat_str->file - start - 1),dat_str->phrase);
            dat_str->file++;
            i++;
        }

    }
    return NULL;
}


void inputFile::start_processing(common& com , searchable& phrases){
    pthread_create(&thread1,NULL,thread_proc,&trd);
    //trd.fil_obj = this;
    // Set the char pointer for the thread

    char * temp = file; // pointer to memory mapped file
    temp += file_size/2;
    //cout.setf(std::ios::unitbuf);
    int bytes = temp - file;

    while(*temp != '\n'){
        bytes++;
        temp++;
    }

    if(*temp == '\n'){
        temp++;

        pthread_mutex_lock(&(trd.mutex));
        trd.thread_indicator = 1;         // signalling variable
        trd.file = temp;
        trd.phrase = phrases.text_to_search[0];
        trd.start_bytes = bytes + 1;     // the start pointer of the mapped file
                                                 // for the second thread.
                                                 // i.e second thread will start processing                         from this pointer
        pthread_cond_signal(&(trd.cond));
        pthread_mutex_unlock(&(trd.mutex));

        // Now process half of the file and put the results in record vector

        temp--; // this thread will process upto '\n' first half of the file
         }
}

Please let me know where have I made a logical flaw or have I implemented the condition variables in a wrong way.

Also, I have initialized both mutex and the condition variable as pthread_mutex_init(&(trd.mutex),NULL) and pthread_cond_init(&(trd.cond),NULL) instead of PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER respectively. Would that be a reason of my problem?

Thanks.

6
  • A slightly silly question, but is trd->thread_indicator initialized to 0 anywhere? Commented Jul 18, 2011 at 18:53
  • Yes it is initialized in the constructor of the class inputFile and in the constructor itself I have initialized the mutex and the condition variables. Commented Jul 18, 2011 at 18:55
  • Ok, just thought I'd ask because usage of uninitialized variables will make cond_wait be skipped. As an aside, the if statement following the wait seems redundant (unless you can pass a value that isn't 1), and will leave the mutex locked if not entered. Commented Jul 18, 2011 at 19:00
  • Is this one shot processing, as in one call to start_processing, or do you execute start_processing in a loop? where and when is thread1 joined? Commented Jul 18, 2011 at 19:07
  • Yes, start_processing is called only once.I am detaching the thread (commented in the code) so no need to join the thread. Commented Jul 18, 2011 at 19:20

2 Answers 2

3

Your use of the condition variable for sleeping is slightly off, the usual method is:

while(!condition)
{
    pthread_cond_wait(...);
}

This handles the case of spurious wakeups (which may happen). Your current code is not checking any condition, but should probably be changed to something like the following:

while (dat_str->thread_indicator != 1)
{
    pthread_cond_wait(&(dat_str->cond),&(dat_str->mutex));
}
Sign up to request clarification or add additional context in comments.

1 Comment

Initially I had put it inside a while loop, I was getting the same behaviour. I will edit the code here as putting it inside while is the usual practice.
3

Probably not the real problem. But you should be careful you do not always unlock the mutex

    pthread_mutex_lock(&(dat_str->mutex));

    while(dat_str->thread_indicator == 0 )
    {    pthread_cond_wait(&(dat_str->cond),&(dat_str->mutex));
    }

    if(dat_str->thread_indicator == 1)
    {
        // CODE    

        pthread_mutex_unlock(&(dat_str->mutex));

        // MORE CODE
    }
    // What happens if it is not 1? The lock is still held.

    return NULL

What happens if the file does not contain '\n'

while(*temp != '\n') // You may want to test against the last good location.
{
    bytes++;
    temp++;
}

4 Comments

You are right..after putting an else I found sometimes the code is going into the else loop too !!! but i am only assigning two values to the thread indicator...but it is taking some garbage value sometimes, how can that be possible??? But in other cases even when it gets the thread indicator as "1" I am getting wrong print in the thread.
It is a '\n' delimited file.I will always find a '\n' as the file is going to be pretty large.
@ArunMu: Never say always (it will bite you in the arse). Just like the condition above should have always been '1'. Someday something will always go wrong and you need to protect yourself from spinning off to infinity.
:Thanks for the advice..but what could be other problem in the code.I am failing to find one as the while loop is terminating at the correct position :(

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.