Trouble Creating Pointer to Function for use with CreateThread

[partytime4321]

Hi all,

I want to start by apologizing if this is in the wrong forum - I wasn't sure if it would be best put in beginner, windows, or general.

I am having an issue creating a pointer to a function, which I need so I can pass it to CreateThread(). I have never done this before, and am not sure why I am getting errors or how to fix them. I am using the GCC compiler in the MinGW suite.

Here is a snippet of code, where I am having issues:

int NetMain() // Tried with int or 'DWORD WINAPI NetMain', no luck
{
// some networking code here
}
int (*NetEntryPoint) __attribute__((cdecl)); // I have tried replacing 'cdecl' with 'WINAPI', and also leaving out the attribute part entirely
NetEntryPoint = &NetMain;

I get the following errors / warnings with this piece of code:
Line 5: warning: 'cdecl' attribute only applies to function types [-Wattributes]
Line 6: error: 'NetEntryPoint' does not name a type

Any help would be greatly appreciated, I have no idea what I am doing with this part.

Another quick question: Can I share global variables between both threads, or should I use sendmessage() / postmessage()?

Thanks very much,
Ryan

[Disch]

For starters, I recommend using std::thread (if your compiler supports it), as it not only is portable... but all the synchronization object (mutexes, control variables, etc) are easier to understand.

If your compiler does not support it, another alternative is boost, which has a virtually identical API to std::thread. But it requires downloading/installing a separate library.

If none of those are an option or you just would prefer to use WinAPI threads....


The WinAPI thread entry point I believe has to be of the CALLBACK calling convention. It also needs to take a single void pointer as a parameter and return a DWORD:

DWORD CALLBACK ThreadFunc( void* userdata )
{
   // this is your thread
   return 0;
}


// ...
DWORD thread_id;

CreateThread(
   nullptr,   // security attributes for the thread (null to use defaults -- recommended)
   0,  // stack size (null to use defaults -- recommended)
   &ThreadFunc,  // pointer to the entry function
   whatever,  // whatever you want to be passed as 'userdata' to the entry function
   0,  // creation flags, 0 for a normal thread -- recommended)
   &thread_id   // pointer to a var to receive the thread id (optional, can be null if you don't care
  );

Another quick question: Can I share global variables between both threads, or should I use sendmessage() / postmessage()?

SendMessage does not communicate between threads. It directly calls the window proc of whatever window you're contacting. You probably should not call it from another thread unless your window handling code is extremely threadsafe (if you're asking these questions, it isn't)

PostMessage (afaik) is thread safe and ok to call between threads as long as the data you pass is copied in the call. If you are passing a pointer to data (as is often the case in messages), that data can have race conditions if not guarded -- so I don't recommend you do this either.



Global variables can be shared. However access to them must be guarded behind a mutex of sorts to prevent both threads from accessing the variable/object at the same time.

Thread safety is actually quite a complex topic.....

[Disch]

Example of something terrible that you must never do with threads:

std::string stuff;   // global string object

void thread1()
{
   stuff += "some string data";
}

void thread2()
{
  std::cout << stuff;
}

This is bad because it opens up a race condition between threads. Both threads are trying to access 'stuff' at the same time.

Sometimes, you'll get lucky and this will work. Other times you won't, and this will do strange things, like corrupt memory or even crash the program. Serious business! And since it doesn't reproduce reliably, it is VERY HARD to find and fix.

To solve this, there is a concept of a "mutex". Mutexes are an object that a thread "locks". Once one thread has a mutex locked, no other threads are able to lock it. If a thread tries to lock it and it's already locked by another thread... the thread will wait (sleep) until the mutex becomes unlocked.

Basically it guarantees that no two threads are accessing the locked data at the same time.

Fixing the above example with a mutex:

/*
   Note I'm using filler/conceptual names for mutexes here.  This code will not work if you
copy/paste.   I'm just using it to show the concept.

I don't remember WinAPI's name for all this stuff.
*/
std::string stuff;   // global string object
MutexType  mutex;  // The mutex

void thread1()
{
   LockMutex(mutex);   // lock the mutex
   stuff += "some string data";  // now it's safe to modify the global object
   UnlockMutex(mutex);  // now that we're done, unlock it
}

void thread2()
{
  LockMutex(mutex);  // do the same thing here
  std::cout << stuff;
  UnlockMutex(mutex);
}

Since global object access is now behind a mutex, you are guaranteed that threads will only access it one at a time. Thus eliminating the possibility for race conditions.

An even better way to do this is to use some kind of RAII locker. Some kind of class that that locks the mutex in its constructor and unlocks it in its destructor, so you don't have to worry about forgetting to unlock (or worry about an exception exiting the function early, skipping your unlock)

Something like that would look more like this:

void thread1()
{
   RAII_Locker   lock(mutex);  // locks the mutex in the ctor

   stuff += "some string data";
}  // 'lock' goes out of scope here, so dtor is run, unlocking the mutex 

Mutexes are absolutely necessary for multithreaded code and you must use them to guard every variable that is accessed in multiple threads. There are exceptions... and ways to do lock-free data access.... but those are extremely advanced topics that even pros often get wrong when they try to implement. So I recommend that for now, you just accept and stick to the simple rule that "all data shared between threads must be behind a mutex".

[partytime4321]

Thanks very much for your reply... I would prefer to use the Win32 threads if possible (though good to know there are alternatives, I didnt think there were on windows)

When I tried pasting your code in, I got "error initialising arguement 4" from CreateThread (the line quoted was from winbase.h, not the actual program) and I also get an error saying I cant convert const void* to 'PVOID {aka void*}'

Do you know how I can fix those errors? Im assuming 'CALLBACK' means I dont have to explicitly create a pointer to it, and can just use the name with a reference operator, is this correct?

In terms of passing the data, I can probably do all of it just by copying, so I will use PostMessage, thanks.

Thanks very much for your reply,
Ryan

[partytime4321]

I had to do this very slow thing that worked like a mutex for file exchange between 2 programs (very slow, 2s RTT) Unfortunately the other language had no messaging bindings what so ever, and a corrupt networking library (ugh)

Thanks for telling me about mutexes, I used to do it the dangerous way and just hope for the best (in the other language, Ive never done anything with multiple threads in C++)

[Disch]

I cant convert const void* to 'PVOID {aka void*}'

You're trying to pass a pointer to a const object in your code. The pointer must be non-const. I cannot tell you how to fix without seeing the code.

Im assuming 'CALLBACK' means I dont have to explicitly create a pointer to it, and can just use the name with a reference operator, is this correct?

No. The CALLBACK specifies the calling convention. It has to do with how the parameters are passed to/from the function at the assembly level. It's not really a detail work getting into -- just know that it has to be there for this function.

Similar to how WinMain should be int WINAPI WinMain(...

In terms of passing the data, I can probably do all of it just by copying, so I will use PostMessage, thanks.

Just remember that pointers are passed by copy, but the data they point to is not.

So if you are passing any string data or anything else that gets cast to/from a pointer, it likely will not be safe to pass this way.

I used to do it the dangerous way and just hope for the best (in the other language,

Many other higher level languages automate this and hide it from the user. So you might have been ok. But it depends on the language.

C++ is not one of those languages ;P

[partytime4321]

Here is my code below, CreateThread is called on line 198... If you could please take a look at some of it, that would be terrific. sorry for double message, I had a hard time getting it all to fit:

[partytime4321]

#define WIN32_LEAN_AND_MEAN
#define _WIN32_WINNT 0x501
#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <stdlib.h>
#include <iostream>
#include <cstring>
using namespace std;

// Need to link with Ws2_32.lib, Mswsock.lib, and Advapi32.lib
// pragma code was removed and placed in the linker settings instead, since GCC does it that way

#define DEFAULT_BUFLEN 512
#define DEFAULT_PORT "27015"

/*  Declare Windows procedure  */
LRESULT CALLBACK WindowProcedure (HWND, UINT, WPARAM, LPARAM);
DWORD CALLBACK NetMain(char* userdata)
{
    WSADATA wsaData;
    SOCKET ConnectSocket = INVALID_SOCKET;
    struct addrinfo *result = NULL,
                    *ptr = NULL,
                    hints;
    char* sendbuf = "this is a test";
    char recvbuf[DEFAULT_BUFLEN];
    int iResult;
    int recvbuflen = DEFAULT_BUFLEN;

    cout << "Enter the address you would like to connect to." << endl;
    char address[20];
    char* pAddress = &address[0];
    cin >> address;

    // Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if (iResult != 0)
    {
        cout << "WSAStartup failed with error: " << iResult << endl;
        return 1;
    }

    ZeroMemory( &hints, sizeof(hints) );
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    // Resolve the server address and port
    iResult = getaddrinfo(pAddress, DEFAULT_PORT, &hints, &result);
    if ( iResult != 0 )
    {
        cout << "getaddrinfo failed with error: " << iResult << endl;
        WSACleanup();
        return 1;
    }

    // Attempt to connect to an address until one succeeds
    for(ptr=result; ptr != NULL ;ptr=ptr->ai_next) {

        // Create a SOCKET for connecting to server
        ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype,
            ptr->ai_protocol);
        if (ConnectSocket == INVALID_SOCKET)
        {
            cout << "socket failed with error: " << WSAGetLastError() << endl;
            WSACleanup();
            return 1;
        }

        // Connect to server.
        iResult = connect( ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
        if (iResult == SOCKET_ERROR)
        {
            closesocket(ConnectSocket);
            ConnectSocket = INVALID_SOCKET;
            continue;
        }
        break;
    }

    freeaddrinfo(result);

    if (ConnectSocket == INVALID_SOCKET)
    {
        cout << "Unable to connect to server!" << endl;
        WSACleanup();
        return 1;
    }

    // Send an initial buffer for handshake
    iResult = send( ConnectSocket, sendbuf, (int)strlen(sendbuf), 0 );
    if (iResult == SOCKET_ERROR)
    {
        cout << "send failed with error: " << WSAGetLastError() << endl;
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }
    char message [512];
    do
    {
        cin.getline(message,512);
        if (strcmp (message,"quit") == 0) {break;}
        else
        {
            send (ConnectSocket, message, (int)strlen(message),0);
            strcpy (message,"");
        }
    }while (true);

    cout << "Bytes Sent: " << iResult << endl;

    // shutdown the connection since no more data will be sent
    iResult = shutdown(ConnectSocket, SD_SEND);
    if (iResult == SOCKET_ERROR)
    {
        cout << "shutdown failed with error: " << WSAGetLastError() << endl;
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }

    // Receive until the peer closes the connection
    do {

        iResult = recv(ConnectSocket, recvbuf, recvbuflen, 0);
        if ( iResult > 0 )
            cout << "Bytes received: " << iResult << endl;
        else if ( iResult == 0 )
            cout << "Connection closed" << endl;
        else
            cout <<"recv failed with error: " << WSAGetLastError() << endl;

    } while( iResult > 0 );

    // cleanup
    closesocket(ConnectSocket);
    WSACleanup();

    char wait;
    cin >> wait;
    return 0;
}

/* int (*NetEntryPoint) __attribute__((cdecl));
NetEntryPoint = &NetMain; */

/*  Make the class name into a global variable  */
char szClassName[ ] = "Client GUI";

int WINAPI WinMain (HINSTANCE hThisInstance,
                     HINSTANCE hPrevInstance,
                     LPSTR lpszArgument,
                     int nCmdShow)
{
    HWND hwnd;               /* This is the handle for our window */
    MSG messages;            /* Here messages to the application are saved */
    WNDCLASSEX wincl;        /* Data structure for the windowclass */

    /* The Window structure */
    wincl.hInstance = hThisInstance;
    wincl.lpszClassName = szClassName;
    wincl.lpfnWndProc = WindowProcedure;      /* This function is called by windows */
    wincl.style = CS_DBLCLKS;                 /* Catch double-clicks */
    wincl.cbSize = sizeof (WNDCLASSEX);

    /* Use default icon and mouse-pointer */
    wincl.hIcon = LoadIcon (NULL, IDI_APPLICATION);
    wincl.hIconSm = LoadIcon (NULL, IDI_APPLICATION);
    wincl.hCursor = LoadCursor (NULL, IDC_ARROW);
    wincl.lpszMenuName = NULL;                 /* No menu */
    wincl.cbClsExtra = 0;                      /* No extra bytes after the window class */
    wincl.cbWndExtra = 0;                      /* structure or the window instance */
    /* Use Windows's default colour as the background of the window */
    wincl.hbrBackground = (HBRUSH) COLOR_BACKGROUND;

    /* Register the window class, and if it fails quit the program */
    if (!RegisterClassEx (&wincl))
        return 0;

    /* The class is registered, let's create the program*/
    hwnd = CreateWindowEx (
           0,                   /* Extended possibilites for variation */
           szClassName,         /* Classname */
           "GUI Client",        /* Title Text */
           WS_OVERLAPPEDWINDOW, /* default window */
           CW_USEDEFAULT,       /* Windows decides the position */
           CW_USEDEFAULT,       /* where the window ends up on the screen */
           600,                 /* The programs width */
           600,                 /* and height in pixels */
           HWND_DESKTOP,        /* The window is a child-window to desktop */
           NULL,                /* No menu */
           hThisInstance,       /* Program Instance handler */
           NULL                 /* No Window Creation data */
           );

    /* Make the window visible on the screen */
    ShowWindow (hwnd, nCmdShow);
    DWORD threadID;
    CreateThread(0, 10000, &NetMain, 10, 0, &threadID); // Create new thread to handle networking, makes adding the GUI easier, also program will be more fluid when transfering large files (not yet supported)

    /* Run the message loop. It will run until GetMessage() returns 0 */
    while (GetMessage (&messages, NULL, 0, 0))
    {
        /* Translate virtual-key messages into character messages */
        TranslateMessage(&messages);
        /* Send message to WindowProcedure */
        DispatchMessage(&messages);
    }

    /* The program return-value is 0 - The value that PostQuitMessage() gave */
    return messages.wParam;
}




LRESULT CALLBACK WindowProcedure (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
{
    switch (message)
    {
    case WM_CREATE: // GUI element creation goes here, network connection should already be open
        CreateWindow("Edit",
                     "",
                     WS_VISIBLE | WS_CHILD | WS_BORDER,
                     10, 10, 400, 25,
                     hwnd, (HMENU) 1, NULL, NULL);
        CreateWindow("BUTTON",
                     "Send Command",
                     WS_VISIBLE | WS_CHILD | WS_BORDER,
                     420, 10, 150, 25,
                     hwnd, (HMENU) 2, NULL, NULL);
        break;
    case WM_COMMAND:

        break;
        case WM_DESTROY:
            PostQuitMessage (0);
            break;
        default:
            return DefWindowProc (hwnd, message, wParam, lParam);
    }

    return 0;
}

[Disch]

I immediately see 2 problems:

1)
DWORD CALLBACK NetMain(char* userdata)
userdata must be a void pointer (void*). Not a char pointer.


2)
CreateThread(0, 10000, &NetMain, 10, 0, &threadID);

10 is an int, not a pointer. The user data must be a pointer.

Since you do not appear to be using the user data in your NetMain function... you an just pass null for this parameter.

EDIT:

Also.... less than 10K for the stack is very small. You really should probably just use the default stack size (pass 0 for that param). It's best not to mess with that unless you really know what you're doing.

[modoran]

CreateThread must be called like this, according to MSDN example:

DWORD WINAPI MyThreadFunction( LPVOID lpParam );

HANDLE hthread;

hThread = CreateThread( 
            NULL,                   // default security attributes
            0,                      // use default stack size  
            MyThreadFunction,       // thread function name
            0,          // argument to thread function , will be passed to lpParam argument of MyThreadFunction
            0,                      // use default creation flags 
            &threadID);   // returns the thread identifier  

However, it is strongly recommended to avoit CreateThread() and use _beginthreadex() CRT function or strange things will occur.

[Disch]

However, it is strongly recommended to avoit CreateThread() and use _beginthreadex() CRT function or strange things will occur.

Such as?

[modoran]

Such as memory leaks inside CRT, please read this Microsoft article:
http://support.microsoft.com/kb/104641/ro

[Disch]

That's very interesting. I didn't know about that. Thanks for the link.

[partytime4321]

Thanks very much for the help, sorry for not replying earlier I was sick all of yesterday... I will try to get everything working again tonight.

How significant is the memory leak?

I guess i should try to use _beginthreadex instead, but what is the difference besides the memory leak?

One more quick question, if you dont mind.. I just wanted to clarify; threads can use multiple cores right? Or is that only done when you create a process?

Thanks,
Ryan

[Disch]

Yes, multiple threads can run on separate cores.