c++ - Multithreaded application, am I doing it right?

Question

Is this doing 4 threads on an array, and then assigning that array to a cumulative count? I believe it is. I know rand() is not thread safe, I'm going to change that once I know the logic is right.

It's a conglomoration of a few bits of advice on Problems passing array by reference to threads and then c++ multithread array

I know it's not practicing best practice methods, but I'm just trying to get it up and running.

I think I got it running correctly, had to change a for counter variable from x to p, not sure why... also moved the fHolder out of for loop.

my fholder always results in 0, I don't know why. I checked the value of counter...

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

using namespace std;

void myThread0 (void *dummy );
void myThread1 (void *dummy );
void myThread2 (void *dummy );
void myThread3 (void *dummy );

//only needed for shared variables
//CRITICAL_SECTION cs1,cs2,cs3,cs4; // global

int main()
{

    //InitializeCriticalSection(&cs1);
    //InitializeCriticalSection(&cs2);
    //InitializeCriticalSection(&cs3);
    //InitializeCriticalSection(&cs4);

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

    int rNum;

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


    //?
    unsigned threadID;

    //HANDLE hThread;
    HANDLE hThread[4];

    const int size = 100000;

    int array[size];

    srand ( time(NULL) );

    printf ("Runs (uses multiple of 100,000) ");
    cin >> numRuns;

    for (int a = 0; a < numRuns; a++)
     {

            hThread[0] = (HANDLE)_beginthread( myThread0, 0, (void*)(array) );
            hThread[1] = (HANDLE)_beginthread( myThread1, 0, (void*)(array) );
            hThread[2] = (HANDLE)_beginthread( myThread2, 0, (void*)(array) );
            hThread[3] = (HANDLE)_beginthread( myThread3, 0, (void*)(array) );

            //waits for threads to finish before continuing
            WaitForMultipleObjects(4, hThread, TRUE, INFINITE);

            //closes handles I guess?
            CloseHandle( hThread[0] );
            CloseHandle( hThread[1] );
            CloseHandle( hThread[2] );
            CloseHandle( hThread[3] );

        //dump array into calculations
        //average array into fHolder

            for (int p = 0; p < size; p++)
             {
                counter += array[p] == 2 ? 1 : -1;
                //cout << counter << endl;
                //cout << count << endl;
                //cout << p << endl;
                counter = count + counter;

                //divide into an exportable value
                //divides by 1,000,000, because each thread handles 250,000
                //cout << "Value " << x << ": " << array[x] << endl;
            }
            fHolder = counter / size;


        cout << "Final Count: " << counter << endl;
        cout << "fHolder: " << fHolder << endl;
        myfile << fHolder << endl;


    }



}
void myThread0 (void *param)
{
    //EnterCriticalSection(&cs1); //aquire the critical section object

    int *i = (int *)param;

    for (int x = 0; x < 25000; x++)
    {
        i[x] = rand() % 2 + 1;
        //cout << i[x] << endl;
    }
    //LeaveCriticalSection(&cs1); // release the critical section object

}

void myThread1 (void *param)
{
    //EnterCriticalSection(&cs2); //aquire the critical section object

    int *i = (int *)param;

    for (int x = 25000; x < 50000; x++)
    {
        //param[x] = rand() % 2 + 1;
        i[x] = rand() % 2 + 1;
        //cout << i[x] << endl;
    }
    //LeaveCriticalSection(&cs2); // release the critical section object

}

void myThread2 (void *param)
{
    //EnterCriticalSection(&cs3); //aquire the critical section object

    int *i = (int *)param;

    for (int x = 50000; x < 75000; x++)
    {
        i[x] = rand() % 2 + 1;
        //cout << i[x] << endl;
    }
    //LeaveCriticalSection(&cs3); // release the critical section object

}

void myThread3 (void *param)
{
    //EnterCriticalSection(&cs4); //aquire the critical section object

    int *i = (int *)param;

    for (int x = 75000; x < 100000; x++)
    {
        i[x] = rand() % 2 + 1;
        //cout << i[x] << endl;
    }
    //LeaveCriticalSection(&cs4); // release the critical section object

}

Answer 1

1. There is a problem with _beginthread, and you should use _beginthreadex instead. Read the documentation of this new function to see how to use it.
2. hThread is array of 4 handles, and so in call to WaitForMultipleObjects there should be 4 instead of 1, and there should be four calls to CloseHandle to close all four of them.
3. Consider renaming that variable ahThread (a for array) or rghThread (rg for range).
4. All four threads are operating on their own pieces of data. And you have the code that deliberately waits for all four thread to finish before actually using those data. This means that you don't need any of four critical sections. There is no danger that some data will be read and written at the same time by two threads.
5. Those critical sections are used wrong. It makes no sense to have only one place in code where a critical section is entered, because the purpose of CS is to prevent some other piece of code to execute while current code under CS is executing. You have only one place in code where CS1 is used, only one where CS2 is used, and so on. Just remove them, because as explained in (4) you don't actually need them.
6. Standard output is a single resource protected by its own internal critical section and is used in all four threads. Because of this all four threads are forced to synchronize on line cout << i[x] << endl; and therefore run slower than necessary.
7. The problem with line cout << i[x] << endl; is that it is not atomic operation and it runs concurrently by four threads. One thread might write its whole line between the writings of i[x] and endl of another thread, which will produce undesired results. I'd remove cout from threads completely.
8. In current revision of the question (revision 3) there is a bug, with for loop being prematurely ended just after CloseHandle. The loop should enclose the inner loop that does the counting.
9. Not thread related, but consider this line of code instead of nine lines of code that currently exist: counter += array[x] == 2 ? 1 : -1;. Variable count is not needed here.
10. All four threads run rather similar code. Consider using parameter param as a pointer to the beginning of the part of the array designated for this thread. This would allow you to have only one function that is run by four threads, with parameter value being different for all four of them.