Calling a virtual function, illegal call of non-static member function?

Started by
24 comments, last by rip-off 10 years, 2 months ago

Hello

I am writing a class that spawns and communicates with a worker thread. The class is going to be Polymorphic such that derived classes can implement their own work functions. This way I can have the thread-safe communication interface (mutex and STL containers) in the WorkerThread class.


class WorkerThread
{
public:
	WorkerThread(void);
	~WorkerThread(void);
private:
	HANDLE hThread;
	static DWORD WINAPI ThreadRoutine(LPVOID param);
	virtual void ThreadFunction(LPVOID param) = 0;     //implemented in derived classes
};

DWORD WINAPI WorkerThread::ThreadRoutine(LPVOID param)
{
	ThreadFunction(param);		//error C2352: 'WorkerThread::ThreadFunction' : illegal call of non-static member function
	return 0;
}

Is this bad? Should I instead pass an object which has its own work function (derived from a base class)? But then I would have to pass some interface to the object.


DWORD WINAPI WorkerThread::ThreadRoutine(LPVOID param)
{
	(somecastings)param->ThreadFunction(/*InterfaceToComm*/);
	return 0;
}
Advertisement

You can't call a member function from a static function, because the member function needs an instance on which to be called. I don't understand the rest of your post.

Should I instead pass an object which has its own work function (derived from a base class)?

Yes.

But then I would have to pass some interface to the object.

No.


DerivedWorkerThread *object = new DerivedWorkerThread("some interface");

CreateThread(...., WorkerThread::ThreadRoutine, (LPVOID)object);

In order to completely encapsulate this in a simple class the thread function has to be static. But now the static function can't call a member function.

Perhaps this is a viable workaround:


class WorkerThread
{
public:
	WorkerThread(void);
	~WorkerThread(void);

	virtual void ThreadFunction(LPVOID param) = 0;
private:
	HANDLE hThread;
	static DWORD WINAPI ThreadRoutine(LPVOID param);
};

WorkerThread::WorkerThread(void)
{
	hThread = CreateThread(NULL, 0, WorkerThread::ThreadRoutine, this, NULL, NULL);
}

DWORD WINAPI WorkerThread::ThreadRoutine(LPVOID param)
{
	static_cast<WorkerThread*>(param)->ThreadFunction(param);
	return 0;
}

I noticed that the compiler now allows for the virtual function to be private, which I think is odd. But I like it since now it is not exposed to the client code.


class WorkerThread
{
public:
WorkerThread(void);
~WorkerThread(void);
private:
virtual void ThreadFunction(LPVOID param) = 0;
HANDLE hThread;
static DWORD WINAPI ThreadRoutine(LPVOID param);
};

I noticed that the compiler now allows for the virtual function to be private, which I think is odd. But I like it since now it is not exposed to the client code.


It's not "odd" at all (and it's not just "now"; C++ has always allowed that). There's one commonly-held belief that all virtual methods should always be private and that the polymorphic behavior of well-encapsulated objects should be an implementation detail of the public interface (though not for "abstract interface" types, of course).

It's handy, too. Even if you have a particular method you know will always need to be overridden in its entirety, having a small non-virtual public wrapper makes it easier to do logging, set breakpoints that capture all calls to the interface, etc. I don't think it should be done in all cases like some people do, but it's good practice in general.

Sean Middleditch – Game Systems Engineer – Join my team!

In this code the constructor creates a thread and passes "this" as a parameter to it. So the ThreadRoutine recieves an object, but how does it know that the object it recieves is its own instance and can start accessing the private members? That's why I think its Odd.


class WorkerThread
{
public:
WorkerThread(void);
~WorkerThread(void);
private:
virtual void ThreadFunction(LPVOID param) = 0;
HANDLE hThread;
static DWORD WINAPI ThreadRoutine(LPVOID param);
int Test;
};

WorkerThread::WorkerThread(void)
{
hThread = CreateThread(NULL, 0, WorkerThread::ThreadRoutine, this, NULL, NULL);
}

DWORD WINAPI WorkerThread::ThreadRoutine(LPVOID param)
{
static_cast(param)->ThreadFunction(param);    //But this is a private function, why is it allowed to be called here?
return 0;
}

Then would it not be pointless to pass anything to ThreadFunction? Because it can directly access all private members anyway? But this does not work out:


class Derived : WorkerThread{
	void ThreadFunction(LPVOID param)
	{
		static_cast<Derived*>(param)->Test = 123;	//If this is Valid:
		Test = 456;					//Then this should also be valid?
	}							//But both gives: error C2248: 'WorkerThread::Test' : cannot access private member declared in class 'WorkerThread'
}; 

I'm puzzled...

Access specifiers are per class, not per object. Since your static TheadRoutine is a member of the WorkerThread class, it has access to everything in any WorkerThread object and can therefore call the private ThreadFunction on the typecast param pointer. Consider what would happen to, for example, the assignment operator or the copy constructor if they could not access private members of the object to copy from.

Thank you. I got the basics working. Will implement thread safe communication asap. Meanwhile, you can have a look at my code:


#pragma once
#include <windows.h>
#include <iostream>
class WorkerThread
{
public:
	WorkerThread(DWORD i):m_quit(false)
	{
		ID = i;
		std::cout << "Constructor called. ID: " << ID << std::endl;
		hThread = CreateThread(NULL, 0, WorkerThread::ThreadRoutine, this, NULL, NULL);
	}
	~WorkerThread(void)
	{
		std::cout << "Exiting thread ID: " << ID << std::endl;
		m_quit = true;
		TerminateThread(hThread, 0);		//unsafe?
	}

protected:
	bool m_quit;
	DWORD ID;
	virtual void ThreadFunction() = 0;     //implemented in derived classes

private:
	HANDLE hThread;
	static DWORD WINAPI ThreadRoutine(LPVOID param)
	{
		static_cast<WorkerThread*>(param)->ThreadFunction();
		return 0;
	}
};

class DerivedA : public WorkerThread
{
public:
	DerivedA(DWORD i) : WorkerThread(i)
	{
	}

private:
	void ThreadFunction()
	{
		while(!m_quit){
			std::cout << "Inside Loop of Derived class A. Thread ID: " << ID << std::endl;
			Sleep(1000+ID*32);
			/*
			Lock mutex
			Get Data
			Unlock mutex

			Computation

			Lock mutex
			Send Data
			Unlock mutex
			*/

		}
	}
}; 

#include <iostream>
#include <vector>
#include <memory>
#include "WorkerThread.h"

int main(){
	std::vector<std::shared_ptr<WorkerThread> > ThreadPool;
	SYSTEM_INFO sysinfo;
	GetSystemInfo( &sysinfo );

	while(true){
	std::cout << "Creating " << sysinfo.dwNumberOfProcessors << " Threads in 5 seconds!" << std::endl;
	Sleep(5000);

	for(DWORD i=0;i<sysinfo.dwNumberOfProcessors;i++)
	{
		ThreadPool.push_back(std::make_shared<DerivedA>(i) );
	}
	std::cout << "Pool size: " << ThreadPool.size() << std::endl;
	Sleep(20000);

	std::cout << ">>>>Clearing Threads<<<<" << std::endl;
	ThreadPool.clear();
	std::cout << ">>>>Clearing Complete<<<<" << std::endl;
	std::cout << "Pool size: " << ThreadPool.size() << std::endl;
	Sleep(5000);
	}

	return 0;
} 

...

...

I'm wondering how I can gracefully exit/terminate a thread without sending it a message to break its main loop.

Using ThreadPool.clear(); all destructors are called and the member m_quit is set true. But if TerminateThread() is not called, the thread will keep on going, because m_quit is no longer in memory, since the object is released. So where m_quit used to be, the thread will read that address and keep on going with its while loop.

But, if I use TerminateThread() then a mutex may remain unlocked or some other members could remain in some undefined state if the thread was working with those members at the time it was terminated.

What happens if I DeleteCriticalSection() on a locked mutex? What if the thread has some pending IO with Win32 during termination? Or it's in the middle of a large memcpy? What other non-blocking safe ways are there for terminating a thread?

What if the thread gets the shared_ptr to itself, which is only deleted when it exits. That way the thread object will remain in memory until it can finish and clean up?

This topic is closed to new replies.

Advertisement