0

i'm doing something for fun, trying to learn multithreading Problems passing array by reference to threads

but Arno pointed out that my threading via process.h wasn't going to be multi-threaded.

What I'm hoping to do is something where I have an array of 100 (or 10,000, doesn't really matter I don't think), and split up the assignment of values to each thread. Example, 4 threads = 250 values per thread to be assigned.

Then I can use this filled array for further calculations.

Here's some code I was working on (which doesn't work)

#include <process.h>
#include <windows.h>
#include <iostream>
#include <fstream>
#include <time.h>
//#include <thread>

using namespace std;

void myThread (void *dummy );

CRITICAL_SECTION cs1,cs2; // global

int main()
{

    ofstream myfile;
    myfile.open ("coinToss.csv");

    int rNum;

    long numRuns;
    long count = 0;
    int divisor = 1;
    float holder = 0;
    int counter = 0;
    float percent = 0.0;

    HANDLE hThread[1000];


    int array[10000];

    srand ( time(NULL) );

    printf ("Runs (use multiple of 10)? ");
    cin >> numRuns;

    for (int i = 0; i < numRuns; i++)
    {
        //_beginthread( myThread, 0, (void *) (array1) );
        //???
        //hThread[i * 2] = _beginthread( myThread, 0, (void *) (array1) );
        hThread[i*2] = _beginthread( myThread, 0, (void *) (array) );

    }
     //WaitForMultipleObjects(numRuns * 2, hThread, TRUE, INFINITE);
     WaitForMultipleObjects(numRuns, hThread, TRUE, INFINITE);

}

void myThread (void *param )
{
    //thanks goes to stockoverflow
    //https://stackoverflow.com/questions/12801862/problems-passing-array-by-reference-to-threads
    int *i = (int *)param;

    for (int x = 0; x < 1000000; x++)
    {
        //param[x] = rand() % 2 + 1;
        i[x] = rand() % 2 + 1;
    }

}

Can anyone explain why it isn't working?

Community
  • 1
  • 1
thistleknot
  • 1,098
  • 16
  • 38

2 Answers2

1

For starters, use _beginthreadex rather than _beginthread, which closes the thread handle on normal run-out. if the thread handle is closed before you begin that WFMO it will likely break immediately since one or more of the handles will be invalid.

Secondly whats with the i*2 on your handle list ? Sending a list of handle to WFMO with every other handle NULL is likely going to error immediately.

Third, WFMO has a maximum wait-list length of 64 threads, so your list of a thousand threads is going to guaranteedly puke as soon as you reach 65 or more.. You just might want to consider limiting that ceiling. The actual value is MAX_WAIT_OBJECTS (or close to that, i can't recall exactly).

And thats all before we even get to the protection of the array you're trying to share.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • I'm just copying a prior example, I have no idea what i*2 is. I have a whole different idea on how I'd like to implement this, and it's not a thread for each element. I'd like to have 4 threads that process up to 100 elements, and then this 100 element array is turned over to main for processing – thistleknot Oct 10 '12 at 02:50
  • the copy was from [this](http://stackoverflow.com/a/12802981/1504523) answer. The i*2 was originally **i * 2 + 1** and makes sense when the line above is not commented out. – Arno Oct 10 '12 at 07:05
0

You should be aware that rand is not thread-safe.

There's even a post about it on SO: Using stdlib's rand() from multiple threads

If you can find yourself a thread-safe random number generator, you would be far better off making a parallel loop using OpenMP, as it maintains a thread pool that is far more efficient than using the thread API.

Otherwise, it might pay to pass a struct into your thread function that gives you the array AND the length required:

struct ArraySlice
{
    ArraySlice( int *arr, size_t start, size_t end)
        : pBegin( arr + start )
        , pEnd( arr + end )
    {}

    int *pBegin, *pEnd;
};

Then to create your threads...

size_t arrayLength = 1000000;
size_t sliceLength = arrayLength / numRuns;

for (size_t i = 0; i < numRuns; i++)
{
    size_t start = i * sliceLength;
    size_t end = min( start + sliceLength, arrayLength );
    ArraySlice *slice = new ArraySlice(array, start, end);
    hThread[i] = (HANDLE)_beginthread( myThread, 0, (void*)slice );
}

And in your thread function:

void myThread (void *param )
{
    ArraySlice *slice = (ArraySlice*)param;
    if( !slice ) return;

    for( int *pos = slice->pBegin, *end = slice->pEnd; pos != end; pos++ )
    {
        *pos = rand();  // Except you need a thread-safe rand() function...
    }

    delete slice;
}
Community
  • 1
  • 1
paddy
  • 60,864
  • 6
  • 61
  • 103
  • g:\dev\threaded.cpp||In function 'int main()':| g:\dev\threaded.cpp|57|error: invalid conversion from 'long unsigned int' to 'void*'| ||=== Build finished: 1 errors, 0 warnings ===| – thistleknot Oct 10 '12 at 02:32
  • 1
    That appears to be an error message. It's not much use to me in that form, but it ought to be useful to you. – paddy Oct 10 '12 at 02:40
  • that's this line "hThread[i] = _beginthread( myThread, 0, (void*)slice );" – thistleknot Oct 10 '12 at 02:44
  • ANSWER: hThread[i] = (HANDLE)_beginthread( myThread, 0, (void*)slice ); via http://www.cplusplus.com/forum/beginner/49578/ – thistleknot Oct 10 '12 at 02:57
  • 1
    Fair enough. Since you didn't cast it in the code you provided, I figured your compiler was happy. – paddy Oct 10 '12 at 03:26