I have inherited a C++ Win32 thread class which roughly works like this. I am looking for ways to improve the design; specifically I am looking for a way to get rid of the spurious Resume() call at the end of initialization. Is there a way this can be automated or omitted?

Base class

class CThread {
public:
this,
dwCreationFlags,
NULL,
0,
{}

protected:

private:
LPVOID pParam = NULL,
DWORD dwCreationFlags = 0,
LPSECURITY_ATTRIBUTES pSecurityAttr = NULL,
DWORD dwStackSize = 0,
{
}

{
}
};


So now when I make a subclass to start the thread, I need to start it suspended because otherwise it might run without being fully initialized, and then resume it after initialization:

class CCreateRestorePointThread
{
zRestorePointEventType eventType,
zRestorePointType      restorePointType)
/* I think I need to start suspended this so that initialization can take place */
, m_bRet(FALSE)
, m_isFinished(false)
{
/* Initialize Restore Point System, structures etc. */
this->Resume(); /* I wanna get rid of this line */
}

private:
DWORD Execute()
{
/* Create Restore Point using class members */
return m_stateMgrStatus.nStatus;
}
};

• Please try to make the example compilable. You never declare or define CThread::Execute which is kind of important. In this case I do not trust myself to advice things without trying them first.
– nwp
Nov 13 '14 at 19:42
• C++ now has threads built-in. You should probably be using the standard library version. It will call the platform/OS specific version behind the Scenes. Nov 13 '14 at 21:55
• It is somewhat difficult to review this. Without providing further details, such as the purpose of this class and an example driver, I can only say that this is probably a bad design. I see at least two resources being managed (Thread handle and DLL hmodule). I do not see any RAII or cleaning up of these objects. The DLL seems to be loaded in the calling space rather than the thread space. I'm not sure whether that matters or not. I'm guessing Execute is a virtual function, but there's no way to know. In summary, please provide more context. Nov 14 '14 at 19:27

The callback function for threads libraries are usually a C-function pointer. As such the thread library depends on the callback having a C ABI (not a C++ ABI). This means the function you pass into the threading API must be declared:

extern "C" void* XXXX(void*); // Assuming return and take void* parameters.
// Which is basically all you can do with a C API.


Note the extern "C" must be there for it to be a valid C++ program.

You are using a static member function as your callback.

static DWORD WINAPI ThreadProc(LPVOID p)


It may look like it is working but this is just pure coincidence that the ABI for static member function happens to be the same on your platform as the ABI for C functions (there is no requirement in the standard). Nor do any OS that I know give guarantees that they will not change this ABI.

You should declare it like this:

// I also suspect the you are using windows
// They also have some special macro magic you may want to look at to force
// specific calling conventions.:
//
// __cdecl    =>  One of those magic double underscore reserved identifiers.
// http://msdn.microsoft.com/en-us/library/kdzttdcb.aspx
//
extern "C" DWORD WINAPI ThreadProc(LPVOID p)
{

• Thanks for your input, but this is not an issue here: My callback function is declared WINAPI which means __stdcall and beginthreadex expects exactly that calling convention. Nov 14 '14 at 15:54