Semafore in Windows problem

[Cybulski]

I'm trying to implement critical section using objects listed in this article: http://www.mycplus.com/tutorial.asp?TID=290

Somehow, it doesn't work like intended, or maybe I do something wrong. Basically my concept looks like this:

DirectoryReader::DirectoryReader(char* p_sConfigPath)
{
GetConfig(p_sConfigPath);
   if( hReaderMutex = CreateMutex( NULL, FALSE, "ReaderMutex") )
   cout << "Mutex created!" << endl;
}

DirectoryReader::MakeList(string sPath)
{
   While( reading filenames in specified directory )
      {
      WaitForSingleObject( hReaderMutex, INFINITE );
      m_vFilenames.push_back( file name );
      ReleaseMutex( hReaderMutex );
      }
}

For some reason blocking doesn't work, for example if I call WaitForSingleObject(); right after cerating mutex (it should put semaphore down), I can still access it later in MakeList method. Do I need pass some options in creating mutex? I don't really know how there security attributes work.
Any ideas?

[edit] I want call MakeList() in multiple threads later, I expect problems too. I also don't want use MFC, since I already started project in WIN32, adding MFC class is impossible it seems.

[Ancient Dragon]

Mutexs work with multiple threads or multiple processes. What you describe is not a problem if there are no other threads or processes that want access to the mutex at the same time. Add a Sleep() before the WaitForSingleObject() to allow time for other threads to gain access to the mutex and see if the wait function blocks until the other thread(s) have released the mutex.

[Cybulski]

I'm pretty sure semaphore is not working correctly anyway. There should be no difference between single thread and more threads acquiring semaphore. For example, if I call WaitForSingleObject() twice in row, thread should be locked, but it isn't. Semaphore represents resource.
I need to read more about parameters of these functions when I get back home, but MSDN is pretty hard lecture for me. I must say POSIX is far easier platform for multitasking programming.

[Ancient Dragon]

>>There should be no difference between single thread and more threads acquiring semaphore
Maybe you don't think so, but there is a big difference.

Its working correctly -- Read the MSDN description for that function and you will see that it doesn't block after the first call when called from within the same thread.

The thread that owns a mutex can specify the same mutex in repeated wait function calls without blocking its execution

[Cybulski]

I am still stuck with this... It seems bit complicated to fill security attributes and security descriptor structures. Anyone have experience with this? Option is to use MFC, but I'm not familiar with GUI controls. Anyway, is it possible to add MFC class to WIN32 project? Seems not.

[mitrmkar]

Most probably the problems are not about security attributes but about you not using threads. Ancient Dragon made a good point above.
Think what would happen if the wait call would block when you are doing it from the same thread that created the mutex in the first place (and you are not having any other threads operating on the mutex).

I think you can forget the security attributes for now and try grasping the basic understanding how the mutexes work. You can use the mutexes also with NULL security attributes, so you don't have to wrestle with those attributes.

[Cybulski]

Yeah sorry I missed point. I'm still thinking about semaptores in POSIX manner - acquired means it's acquired regardless of process accessing it.

[Cybulski]

Ok, I got working threads, but still can't make proper synchronization. It's hard to find good example.

So, there is header file:

class MultiReader :
	public DirectoryReader
{
public:
	MultiReader(char* p_sConfigPath);
	~MultiReader(void);

	static const HANDLE hReaderMutex;
	void CreateThreads(void);
	static DWORD WINAPI ReaderThread(PVOID p_pVoid);
};

And that's how I'm trying synchronize threads:

MultiReader::MultiReader(char* p_sConfigPath):DirectoryReader(p_sConfigPath)
{
	hReaderMutex = CreateMutex( NULL, TRUE, "ReaderMutex");
}

MultiReader::~MultiReader(void)
{
}

void MultiReader::CreateThreads()
{
	unsigned int nThreadNo;

	for( nThreadNo = 0; nThreadNo < vDirectories.size(); nThreadNo++)
	{
		CreateThread( NULL, 0, MultiReader::ReaderThread, (void*)vDirectories.at(nThreadNo).sDirectoryName.c_str(), 0, NULL);
	}
}

DWORD WINAPI MultiReader::ReaderThread(LPVOID p_pVoid)
{
	WaitForSingleObject( hReaderMutex, 0);
	cout << "Thread: " << (char*)p_pVoid << endl;
	ReleaseMutex( hReaderMutex );
	return NULL;
}

I get this:

error LNK2001: unresolved external symbol "public: static void * MultiReader::hReaderMutex" (?hReaderMutex@MultiReader@@2PAXA)
C:\Documents and Settings\hp\My Documents\Visual Studio 2008\Projects\FileFactory\Debug\FileFactory.exe : fatal error LNK1120: 1 unresolved externals

It is after taking compiler advice to make hReaderMutex handle static. I don't want it static too! Class will be part of singleton.

[mitrmkar]

hReaderMutex is not defined anywhere, hence the error. To get over this problem you need something like ...

// in MultiReader.cpp ...
const HANDLE MultiReader::hReaderMutex = CreateMutex( NULL, TRUE, "ReaderMutex");

However, this is a bit bad construct, because when the CreateMutex() fails, you will not know the reason for the failure (GetLastError()).

[Cybulski]

Ok I figured out that static const must be initialize within class body, but still:

class MultiReader :
	public DirectoryReader
{
public:
	MultiReader(char* p_sConfigPath);
	~MultiReader(void);

	static const HANDLE hReaderMutex = CreateMutex( NULL, TRUE, "ReaderMutex");
};

Gives me this:
error C2864: 'MultiReader::hReaderMutex' : only static const integral data members can be initialized within a class

[mitrmkar]

Initialize outside the class i.e.

// outside your class ...
const HANDLE MultiReader::hReaderMutex = CreateMutex( NULL, TRUE, "ReaderMutex");

[Cybulski]

I replaced that Mutex with Semaphore object. There should be no much difference, but anyway, this is pretty much my code now:

// MultiReader.h
class MultiReader :
	public DirectoryReader
{
public:
	MultiReader(char* p_sConfigPath);
	~MultiReader(void);

	static const HANDLE hReaderSemaphore;
	void CreateThreads(void);
	static DWORD WINAPI ReaderThread(PVOID p_pVoid);
};

// MultiReader.cpp

const HANDLE MultiReader::hReaderSemaphore = CreateSemaphore( NULL, 1, 1, NULL);

void MultiReader::CreateThreads()
{
	unsigned int nThreadNo;
	for( nThreadNo = 0; nThreadNo < vDirectories.size(); nThreadNo++)
	{
		CreateThread( NULL, 0, MultiReader::ReaderThread, (void*)&vDirectories.at(nThreadNo), 0, NULL);
	}
} //it works and these variables are defined in derived class

WORD WINAPI MultiReader::ReaderThread(LPVOID p_pVoid)
{

	WaitForSingleObject( hReaderSemaphore, 0);
	
	OPTIONS* ThreadData = (OPTIONS*) p_pVoid;
	cout << ThreadData->sDirectoryName << endl;
	cout << ThreadData->uOperationId << endl;
	DisplayVector(ThreadData->vExtensions);
	cout << endl;

	ReleaseSemaphore( hReaderSemaphore, 1, NULL );
	return NULL;
}

I can't figure out why threads aren't synchronized. I can comment out Wait function, Release function. I can give semaphore initial value of 0 and still get same results. Console output is random. This starts frustrating me.

[mitrmkar]

You should (really) check the return value of WaitForSingleObject() and react accordingly. And also understand what it means to use zero as the time-out interval with WaitForSingleObject().

[Cybulski]

Wow... Thanks for this tip.

I still think of these object in POSIX manner, where (as far as I remember) wait functions do not return untill they acquire semaphore, if timeout was set to infinite. These must be checked in loop... It is pretty much working now.

[Cybulski]

Ok, it seems I must still learn a lot about windows threading.

I have these threads running, but I need them to use base class method and container. Base to MultiReader (DirectoryReader) have method MakeList(string) and vector<string>.

Each threadmust call MakeList and access vector in base class to insert data there. I have declared MakeList(string) as static (do i have to do this?), but now compiler says "left of .push_back must have class/struct/union" and points this at MakeList method.

So basically I'm not done yet with threading and still need help. Any ideas?

[Ancient Dragon]

static methods can only access static class objects. So if that vector was not declared static then the threads can't access it. This has nothing to do with threads and everything to do with c++ classes.

[Cybulski]

When I make vector static, I get unresolved external error.

In meantime I found this:
http://homepages.tesco.net/J.deBoynePollard/FGA/class-function-members-cannot-be-thread-functions.html

Do I have to rebuild my solution?

[Ancient Dragon]

static data objects must also be declared something like global objects

// *.h file
#include <vector>
#include <string>
class foo
{
public:
   static std::vector<std::string> lst;
   // other stuff not shown
};

// *.cpp file
#include "foo.h"
std::vector<std::string> foo::lst;

int main()
{
    return 0;
}

In meantime I found this:
http://homepages.tesco.net/J.deBoyne...functions.html

Only static class methods can be used as thread functions and that link explains why. You already made the method static so don't worry about that part.