segmentation fault

[Agni]

This code here is giving segmentation fault. Any clues why? also any general tips for writing good multi-threaded code are welcome.

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

using namespace std;

unsigned __stdcall Display(void* p)
{
	Sleep(500);
	cout << "Display" << endl;
	cout << *((int*)p) << endl;
	return 0;
}
int main()
{
	unsigned taddr = NULL;
	unsigned createFlag = 0;

	for(int i=0;i<5;i++)
	{
		if (_beginthreadex(NULL,0,Display,&i,createFlag,&taddr))
		{
			cout << "success" << endl;
		}
		else
			cout << "failed" << endl;
	}

	cout << "main's execution is over" << endl;
	ExitThread(0);
}

[Ancient Dragon]

>>ExitThread(0);
delete that line in main()

[Agni]

>>ExitThread(0); delete that line in main()

But if i do that the threads never execute as 'main' terminates and all the threads die. Any reason why you think it might help?

I found a possible solution to the problem, since the threads would sleep in 'Display' and by the time they wake up, main exits hence 'i' doesn't exist anymore and when i try to print the value it crashes. Actually i executed it once without the 'sleep' and it went through fine. Though i think it might still crash some other time when main exits before any of the threads is left incomplete.

[unbeatable0]

It doesn't matter that 'i' doesn't exist when main() ends and the thread is using it, simply because when main() ends the whole program shuts down, including the threads.

[Agni]

It doesn't matter than 'i' doesn't exist when main() ends and the thread is using it, simply because when main() ends the whole program shuts down, including the threads.

The threads wont shut down because I'm using 'ExitThread', if I'm not wrong its the equivalent of pthread_exit for Unix.

[Ancient Dragon]

ExitThread () belongs inside a thread, not in main().
ExitThread is the preferred method of exiting a thread in C code.

There are other methods available to keep main() alive until the threads terminate on their own.

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

using namespace std;
unsigned __stdcall Display(void* p)
{
	Sleep(500);
	cout << "Display" << endl;
	cout << *((int*)p) << endl;
	return 0;
}
int main()
{
	unsigned taddr = NULL;
	unsigned createFlag = 0;
    HANDLE handles[5] = {0};
 	for(int i=0;i<5;i++)
	{
		if (handles[i] = (HANDLE)_beginthreadex(NULL,0,Display,&i,createFlag,&taddr))
		{
			cout << "success" << endl;
		}
		else
			cout << "failed" << endl;
	}
    WaitForMultipleObjects(5, handles, TRUE, INFINITE);

	cout << "main's execution is over" << endl;
}

[Salem]

> Any clues why?
Because you passed a POINTER to i.
The main() thread exits, the i variable goes out of scope and another thread is looking down the barrel of a segfault.

> also any general tips for writing good multi-threaded code are welcome.
Well the first step is to make sure that whatever parameter you pass has a lifetime which exceeds the thread creator.

Using dynamic memory, and then making the thread the owner of that memory is one way to achieve this.
This also solves the other problem of helping to ensure that only one thread has access to the memory passed into it.