1


I'm trying to pass a structure to my thread using CreateThread() and here is my structure :

struct Secure
{
  int UID;
  LPVOID MainClass;
};

And here is the way I call CreateThread()

Secure m_Secure = {Room->g_User[PlayerNumber].UID,this};

CreateThread(NULL,NULL,(LPTHREAD_START_ROUTINE)SecureThread,&m_Secure,NULL,NULL);

Which the first one is integer and the second one is a pointer to the current class.
And, here is my thread and I think that here is the problem

HRESULT WINAPI SecureThread(LPVOID Param)
{
    int UID = -1, UserNumber, i;

    Secure* m_Secure = (Secure*)Param;

    UID = m_Secure->UID;

    CGGCBotDlg *h_MainClass = (CGGCBotDlg*)m_Secure->MainClass;

    if (UID == -1) return 0;

    Sleep(25000);

    for (i = 0; i < TOTAL_CLIENTS; i++)
    {
        if (h_MainClass->Room->g_User[i].UID == UID)
        {
            UserNumber = i;
            break;
        }
    }

    if( h_MainClass->Room->g_User[UserNumber].IsFree == false && h_MainClass->Room->g_User[UserNumber].Secured == false)
        h_MainClass->Room->Kick(h_MainClass->Room->g_User[UserNumber].UID,"Didn't Authorized");

    return 0;
}

When ever this thread is created the program crashes and here is the exception:

First-chance exception at 0x00EC3548 in GGCRoomServer.exe: 0xC0000005: Access violation reading location 0x5D00009C.
Unhandled exception at 0x00EC3548 in GGCRoomServer.exe: 0xC0000005: Access violation reading location 0x5D00009C.
Shahriyar
  • 1,483
  • 4
  • 24
  • 37
  • 3
    your m_Secure variable seems to be local, so it will go out of scope, and the thread is left with a pointer to stack memory that is reused for something else. You need `Secure * m_secure = new Secure();` and you of course need to `delete m_Secure;` before the thread finishes – Erik Sep 26 '13 at 09:21
  • @Erik Whats the point of creating a new Secure structure ? I want to get the structure that I have passed to my thread – Shahriyar Sep 26 '13 at 09:25
  • 1
    The structure only exists as a local variable. It will no longer exist after the function calling CreateThread finishes. So if the thread is alive longer than the function calling CreateThread, you must provide it with a variable that the thread code can delete when it is finished. – Erik Sep 26 '13 at 09:27

2 Answers2

2

Create a heap variable to hold your data and pass that to the thread.

Secure * m_Secure = new Secure();
m_Secure->UID = g_User[PlayerNumber].UID;
m_Secure->MainClass = this;
CreateThread(NULL,NULL,(LPTHREAD_START_ROUTINE)SecureThread,m_Secure,NULL,NULL);

Get the data in the thread, and delete when done

RESULT WINAPI SecureThread(LPVOID Param)
{
    int UID = -1, UserNumber, i;

    Secure* m_Secure = (Secure*)Param;
....
    delete m_Secure;
    return 0;
}
Erik
  • 88,732
  • 13
  • 198
  • 189
2

It looks like you are passing a local variable's address to a thread here

Secure m_Secure = {Room->g_User[PlayerNumber].UID,this};    
CreateThread(NULL,NULL,(LPTHREAD_START_ROUTINE)SecureThread,&m_Secure,NULL,NULL);

Being a local variable, m_Secure wil get out of scope and will be destroyed after the function finishes executing. Furtermore, m_Secure is likely created on the stack. Passing one thread's stack variable address to another is usually a bad idea
You need to create the variable on the heap
Something like

CreateThread(NULL,NULL,(LPTHREAD_START_ROUTINE)SecureThread,new Secure(...),NULL,NULL);
                                                            ^^^^^^^^^^^^^^

And don't forget to delete the pointer afterwards

spiritwolfform
  • 2,263
  • 15
  • 16
  • `new Secure(...)` How should i set data to structure ? this thread creates every second and every thread takes 30 seconds to complete and for every thread the structure that is passes to thread have different data – Shahriyar Sep 26 '13 at 10:42
  • set data any way you like. You may use a constructor, for example (that's what i meant with (...)) – spiritwolfform Sep 26 '13 at 14:44