CODEWITHSUNDEEP
c++linuxmultithreading
Genelia D'souza
Genelia D'souza

Reputation: 125

Thread synchronization issue

I have been given a task of copying millions of folders individually through threading in C/C++ on Linux (with some legacy code).

The individual functionalities like scanning of folders, copying of file from source to destination folder are in place and works properly when executed in serial. The whole issue comes in when done via threading.

Issue: The code behaves a little differently everytime I execute it. Sometimes it copies the entire list of folders proeprly but sometimes it fails.

A sample failing output is:

foldername is [Junk] tCount is 2 cntr is 3

val of folder is Junk tid is 3055356816

foldername is [Notes] tCount is 3 cntr is 4

val of folder is  tid is 3042966416

Folder creation failed /var/anshul/work/copyDirectoryThreaded/test_copied/email/

In the function thread the value of argument passed becomes NULL. In above case the argument Notes is been passed from main function to the thread function, but in thread function the value recieved is NULL.

My main code looks like this:

int main()
{
    int cntr=0;
    int Val = 3;

    tCount = 0;
    pthread_t thread[folderCount]; // folderCount is total number of folders scanned
    int  iret[folderCount];
    std::map<std::string,int>::iterator mFolderIt; // mFolder map contains the folder list.
    char foldername[30] = {0};

    for ( mFolderIt=mFolder.begin() ; mFolderIt != mFolder.end(); )
    {
        if(tCount < Val)
        {
            pthread_mutex_lock( &mutex_tCount );
            tCount++;
            pthread_mutex_unlock( &mutex_tCount );

            sprintf(foldername,"%s",(*mFolderIt).first.c_str() );
            fprintf(stderr, "foldername is [%s] tCount is %d cntr is %d\n",foldername,tCount,cntr);
            iret[cntr] = pthread_create( &thread[cntr], NULL,folderCopy , (void*)foldername);
            cntr++;
            usleep(1); // most crucial for threading.......
            mFolderIt++;
            memset(foldername,0,30);
        }
        else
        {
            while(tCount >= Val)
            {
                usleep(10);
            }
        }
    }

    for(int x = 0 ; x<folderCount ;x++)
        pthread_join(thread[x],NULL);

    return 1;
}

Thread function code:

void * folderCopy(void *f)
{
    fprintf(stderr,"val of folder is %s tid is %u\n", folder, (unsigned int)pthread_self());

    pthread_mutex_lock( &mutex_tCount );
    tCount--;
    pthread_mutex_unlock( &mutex_tCount );
    pthread_exit(NULL);
    return NULL;
}

Can someone please help me to solve this issue.

Upvotes: 1

Views: 318

Answers (2)

Loki Astari
Loki Astari

Reputation: 264351

The pointer passed as the fourth parameter is not thread safe.

pthread_create( &thread[cntr], NULL,folderCopy , (void*)foldername);

So every thread is receiving the same pointer and since the main thread is overwritting the content of this pointer so there is no way to know what content any particular thread sees.

You need to make a private copy of the content for each thread (then let the thread clean it up).

Upvotes: 1

bdonlan
bdonlan

Reputation: 231073

                    fprintf(stderr, "foldername is [%s] tCount is %d cntr is %d\n",foldername,tCount,cntr);
                    iret[cntr] = pthread_create( &thread[cntr], NULL,folderCopy , (void*)foldername);
                    cntr++;
                    usleep(1); // most crucial for threading.......
                    mFolderIt++;
                    memset(foldername,0,30);

There is no guarentee that that usleep will be enough time. Instead, you should make absolutely sure that the memory will remain valid until the other thread has had a chance to use it. The simplest way to do this is to give the other thread its own copy of the data:

iret[cntr] = pthread_create(&thread[cntr], NULL, folderCopy, strdup(foldername));
// ...

void * folderCopy(void *f)
{
    char *folderName = (char *)f;
    // do something with folderName
    free(f);
    return NULL;
}

There are other ways to ensure that the thread has taken a copy of the data, but this is the simplest to get right.

Upvotes: 3

Related Questions