Unhandled exception / Access violation writing location in a Mutex example

Question

I'm working through an example of protecting a global double using mutexes, however I get the error -

Unhandled exception at 0x77b6308e in Lab7.exe: 0xC0000005: Access violation writing location 0x00000068.

I assume this is related to accessing score? (The global double)

#include <windows.h>
#include <iostream>   
#include <process.h>

double score = 0.0; 


HANDLE threads[10];     

CRITICAL_SECTION score_mutex; 


unsigned int __stdcall MyThread(void *data)
{
    EnterCriticalSection(&score_mutex);
    score = score + 1.0; 
    LeaveCriticalSection(&score_mutex); 

    return 0;
}

int main()
{
    InitializeCriticalSection(&score_mutex); 

    for (int loop = 0; loop < 10; loop++)
    {

        threads[loop] = (HANDLE) _beginthreadex(NULL, 0, MyThread, NULL, 0, NULL); 
    }

    WaitForMultipleObjects(10, threads, 0, INFINITE); 

    DeleteCriticalSection(&score_mutex); 

    std::cout << score; 

    while(true);

}

Update:

After fixing the problem with the loop being set to 1000 instead of 10, the error still occured, however when I commented out the pieces of code referring to the mutex the error did not occur.

CRITICAL_SECTION score_mutex; 
EnterCriticalSection(&score_mutex); 
LeaveCriticalSection(&score_mutex); 
InitializeCriticalSection(&score_mutex); 
DeleteCriticalSection(&score_mutex); 

Update 2

The threads return 0 as per convention (It's been a long week!)

I tried adding back in the mutex-related code, and the program will compile and run fine (other than the race condition issues with the double of course) with CRITICAL_SECTION, InitializeCriticalSection and DeleteCriticalSection all added back in. The problem appears to be with EnterCriticalSection or LeaveCriticalSection, as the error reoccurs when I add them.

Plasterwork

Note: by convention, thread return code is 0 if it terminates without errors. – Shulock

Have you checked the return value from any of the invoked functions? Always check error codes from C API functions. It's the first step you should take, especially when you get a crash! – Shulock

Thank you - I should have thought - it's been a long week! – Plasterwork

Found your bug, check out my answer. – Shulock

@ShimmerGeek: You don't need a busy loop while(true); at the end. WaitForMultipleObjects waits for all the secondary threads so that the primary thread doesn't exit. – Tamanaha

It's just to keep the console window open so the score can be viewed onscreen. – Plasterwork

Answer 1

The remaining bug in your code is in the call to WaitForMultipleObjects(). You set the 3rd parameter to 0 (FALSE) such that the main thread unblocks as soon as any of the 10 threads finishes.

This causes the call to DeleteCriticalSection() to execute before all threads are finished, creating an access violation when one of the (possibly) 9 other threads starts and calls EnterCriticalSection().

Herthahertz

Oh curses, I actually had that producing a different bug, and set it to TRUE (in a different example) and then promptly forgot about it! Thank you so much :) – Plasterwork

With this answer, it's good to know that calling DeleteCriticalSection() causes access violation on another thread at EnterCriticalSection(). It's of course easy to understand, but if an access violation appears first, maybe not. – Pittman

Answer 2

You're writing beyond the end of your threads[10] array:

for (int loop = 0; loop < 1000; loop++){
     threads[loop];
}

threads only has size 10!

Marchelle

How the hell... Wow, okay, that's fixed! Still getting the error though :) – Plasterwork

I think you need to initialise your critical section object: score_mutex with InitializeCriticalSection, too. – Marchelle

Oh, missed that. No everything looks okay now. If you run in the debugger which line does the error occur on? – Marchelle

Answer 3

Your problem is that WaitForMultipleObjects is not waiting for all the threads to complete, causing the critical section to be prematurely deleted. According to MSDN, the third argument is

bWaitAll [in]

If this parameter is TRUE, the function returns when the state of all objects in the >lpHandles array is signaled. If FALSE, the function returns when the state of any one of >the objects is set to signaled. In the latter case, the return value indicates the object >whose state caused the function to return.

You set this to 0, which returns when ANY ONE of your threads completes. This causes the following DeleteCriticalSection to be run while there's still threads waiting to access it.

Drobman

Yeah, silly mistake - fixed now, thanks. Error still happening though. – Plasterwork

Answer 4

You should also declare score as a volatile so you don't have cached value problem.

Cedilla