Multi-threaded mayhem

This is a discussion on Multi-threaded mayhem within the C Programming forums, part of the General Programming Boards category; I'm programming a multi-threaded, simple port scanner to try and familiarize myself with multi-threading, and I've run into a problem. ...

  1. #1
    Ex scientia vera
    Join Date
    Sep 2007
    Posts
    478

    Multi-threaded mayhem

    I'm programming a multi-threaded, simple port scanner to try and familiarize myself with multi-threading, and I've run into a problem. I'm passing a pointer to a struct(Allocated on the heap) as the argument, then dereferencing the pointer in the thread callback and trying to use it to pass info to the thread, such as IP, ports and so on. However, I *think* the threads are segfaulting when it tries to dereference the pointer, but I can't really debug it, as I've never debugged threads and even when I tried doing so with gdb in codeblocks, it seemed to return fine.

    The way it 'doesn't work' is basically by not even printing the sPort and ePort variables, or anything else after that, as if it's shut down.

    Here's the code:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <windows.h>
    #include <process.h>
    
    #define ecs() EnterCriticalSection(&cSection);
    #define lcs() LeaveCriticalSection(&cSection);
    
    struct sInfo
    {
        unsigned int   startPort;
        unsigned int   noPorts;
        char           *host;
    };
    
    CRITICAL_SECTION cSection;
    
    __stdcall void scanThread(void * Param)
    {
        struct sockaddr_in remoteHost;
    
        struct sInfo sinfo  = *((struct sInfo*)Param); // Dereferencing the pointer. Wrong ? Cast to struct pointer, dereference pointer?
    
    
        int sPort = sinfo.startPort,
                    ePort = sPort + sinfo.noPorts,
                    theSocket;
    
        WSADATA wsaData;
    
        WSAStartup(MAKEWORD(2,0), &wsaData);
    
    
        ecs();
    
        printf("Starting port: &#37;d\n", sPort);
    
        lcs();
    
    
    
        remoteHost.sin_family = AF_INET;
        if ((remoteHost.sin_addr.s_addr = inet_addr(sinfo.host)) == -1)
        {
            ecs();
    
            printf("Could not resolve host.\n");
    
            lcs();
    
            _endthreadex(0);
        }
    
        memset(remoteHost.sin_zero, 0, 8);
    
        theSocket = socket(AF_INET, SOCK_STREAM, 0);
    
        if (theSocket == INVALID_SOCKET)
        {
            ecs();
    
            printf("Could not create socket: %d\n", WSAGetLastError());
    
            lcs();
    
            _endthreadex(0);
        }
    
    
    
    
        for (; sPort < ePort; sPort++)
        {
            remoteHost.sin_port = htons(sPort);
    
    
            if (connect(theSocket, (struct sockaddr*)&remoteHost, sizeof(struct sockaddr)))
            {
                ecs();
    
                printf("Port %d: CLOSED\n", sPort);
    
                lcs();
            }
            else
            {
                printf("Port %d: OPEN\n", sPort);
    
            }
        }
    
    
        closesocket(theSocket);
        WSACleanup();
    
        _endthreadex(0);
    }
    
    
    int main(int argc, char  **argv)
    {
        if (argc < 4)
        {
            printf("Usage:\n"\
                   "\tportscanner IP STARTPORT ENDPORT [NO.THREADS]\n\n");
    
            return 0;
        }
    
        int startPort = atoi(argv[2]), endPort = atoi(argv[3]),
                        portsPerThread, noThreads = 5, i = 0, x = 0;
    
    
    
        HANDLE *threadHandles;
    
    
        if (startPort > endPort)
        {
            printf("The start port must be smaller than the end port.\n");
    
            return 0;
        }
    
        if (argc >= 5) noThreads = atoi(argv[4]);
    
        threadHandles = malloc(noThreads * sizeof(HANDLE));
    
    
        portsPerThread = ((endPort - startPort)+1) / noThreads;
    
    
    
    
        printf("Number of threads: %d\n"
               "Ports per thread:  %d\n", noThreads, portsPerThread);
    
    
        // Array of pointers to struct sInfo.
        struct sInfo *scanInfo[noThreads];
    
    
    
    
    
        InitializeCriticalSection(&cSection);
        for (i = 0; x < noThreads; i += portsPerThread )
        {
            // Allocate memory at each pointer.
            scanInfo[x] = malloc(sizeof(struct sInfo));
            scanInfo[x]->noPorts = portsPerThread;
            scanInfo[x]->host    = malloc(sizeof(char) * 100);
    
            strcpy(scanInfo[x]->host, argv[1]);
    
            scanInfo[x]->startPort = i;
    
    
            threadHandles[x++] = (HANDLE)_beginthreadex(NULL,
                                 0,
                                 scanThread,
                                 (void*)scanInfo[x], // Pass pointer to thread, here.                             0,
                                 NULL);
        }
    
    
        WaitForMultipleObjects(noThreads,
                               threadHandles,
                               TRUE,
                               INFINITE);
    
    
        free(threadHandles);
    
        for(i = 0; i < noThreads; i++)
        {
            free(scanInfo[x]->host);
            free(scanInfo[x]);
        }
    
        //free(scanInfo);
        DeleteCriticalSection(&cSection);
    
    
        return 1;
    
    }
    Last edited by IceDane; 03-23-2008 at 09:05 PM.

  2. #2
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    Won't this:
    Code:
      for (i = 0; noThreads--; i += portsPerThread )
    cause this
    Code:
    WaitForMultipleObjects(noThreads,
                               threadHandles,
                               TRUE,
                               INFINITE);
    to work differently from what you want?

    Todd
    Mac and Windows cross platform programmer. Ruby lover.

    Quote of the Day
    12/20: Mario F.:I never was, am not, and never will be, one to shut up in the face of something I think is fundamentally wrong.

    Amen brother!

  3. #3
    Ex scientia vera
    Join Date
    Sep 2007
    Posts
    478
    Quote Originally Posted by Todd Burch View Post
    Won't this:
    Code:
      for (i = 0; noThreads--; i += portsPerThread )
    cause this
    Code:
    WaitForMultipleObjects(noThreads,
                               threadHandles,
                               TRUE,
                               INFINITE);
    to work differently from what you want?

    Todd
    Hell yeah it will. That seems to have fixed it, thanks a bunch. Can't believe I missed that.

  4. #4
    Ex scientia vera
    Join Date
    Sep 2007
    Posts
    478
    I've run into a new problem, and I've replaced the updated code with the old code in the original post so I don't make this thread a thousand pages.

    Anyway, I figured out that the threads had issues with synchronization, that is, using the same starting port as they were getting it from global memory, which made a lot of threads use same starting ports and thus ruining the point of the program.

    So I figured I'd create an array of pointers, which I have, and then pass the pointers one by one to the threads, so I'd definitely be making them access separate memory. However, when trying to dereference the pointer in the thread proc, it seems to segfault.

    I'm not too good with pointer arithmetic, when it goes that deep, to be honest, and I'd love it if someone could explain why this isn't working. Also, if you have any links on some more 'advanced' pointer arithmetic, which has detailed instructions on dereferencing n-duple pointers, and some thumb rules, I'd be eternally grateful. I've been googling for some info on it to fix my problem, but none of it was advanced enough to help me out.

  5. #5
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Do WSAStartup & WSACleanup need to be called for each thread? I think it is only once for the entire app.

    You're not cosing the handles to the threads after waiting on them. Since you're freeing them, you may as well close them first.

    Using C99 huh. Interesting choice that you've chosen to use malloc in one case, but a variable sized array in the other place.

    Were you planning on using that critical section anywhere? It doesn't appear as if you need it, so you may as well get rid of it.

    This is a no-no:
    Code:
            threadHandles[x++] = (HANDLE)_beginthreadex(NULL,
                                 0,
                                 scanThread,
                                 (void*)scanInfo[x], // Pass pointer to thread, here.
                                 NULL);
    Look how you're using x twice on that line. Just do the increment in the for-loop instead like this:
    Code:
        for (i = 0; x < noThreads; i += portsPerThread, ++x )
    Note that you had already set i to zero before the for-loop so you're doing it twice which is unnecessary, and inconsistent with what you're doing with x.
    Last edited by iMalc; 03-23-2008 at 11:53 PM.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  6. #6
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,702
    >> __stdcall void scanThread(void * Param)
    should be
    unsigned __stdcall scanThread(void * Param)

    >> ...so you may as well get rid of it.
    It's being used to mutex access to stdout. Nothing wrong with that.

    gg

  7. #7
    Ex scientia vera
    Join Date
    Sep 2007
    Posts
    478
    Quote Originally Posted by iMalc View Post
    Do WSAStartup & WSACleanup need to be called for each thread? I think it is only once for the entire app.

    You're not cosing the handles to the threads after waiting on them. Since you're freeing them, you may as well close them first.

    Using C99 huh. Interesting choice that you've chosen to use malloc in one case, but a variable sized array in the other place.

    Were you planning on using that critical section anywhere? It doesn't appear as if you need it, so you may as well get rid of it.

    This is a no-no:
    Code:
            threadHandles[x++] = (HANDLE)_beginthreadex(NULL,
                                 0,
                                 scanThread,
                                 (void*)scanInfo[x], // Pass pointer to thread, here.
                                 NULL);
    Look how you're using x twice on that line. Just do the increment in the for-loop instead like this:
    Code:
        for (i = 0; x < noThreads; i += portsPerThread, ++x )
    Note that you had already set i to zero before the for-loop so you're doing it twice which is unnecessary, and inconsistent with what you're doing with x.
    Like Codeplug pointed out, I'm using them to synchronize access to stdout so I don't get garbage when I print debug statements. I've macroed EnterCriticalSection(&cSection) to ecs();, and LeaveCriticalSection(&cSection); to lcs(); because it's incredibly annoying to type the whole thing out every time I need to print something.

    Anyway - thanks for the pointer about using x - I was wondering why it said using x in this context was undefined.

    However, have you any idea why it is segfaulting?

  8. #8
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,702
    Code:
            threadHandles[x++] = (HANDLE)_beginthreadex(NULL,
                                 0,
                                 scanThread,
                                 (void*)scanInfo[x], // Pass pointer to thread, here.
                                 NULL);
    I believe iMalc already pointed it out. Every thread is getting an invalid pointer. Which you then dereference with
    >> struct sInfo sinfo = *((struct sInfo*)Param);

    gg

  9. #9
    Ex scientia vera
    Join Date
    Sep 2007
    Posts
    478
    Quote Originally Posted by Codeplug View Post
    Code:
            threadHandles[x++] = (HANDLE)_beginthreadex(NULL,
                                 0,
                                 scanThread,
                                 (void*)scanInfo[x], // Pass pointer to thread, here.
                                 NULL);
    I believe iMalc already pointed it out. Every thread is getting an invalid pointer. Which you then dereference with
    >> struct sInfo sinfo = *((struct sInfo*)Param);

    gg
    Hmm, that seems to be it. I compiled it before and it didn't work. Maybe I didn't compile.

    Thanks for the help guys.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Multi dimensional array
    By $l4xklynx in forum C Programming
    Replies: 7
    Last Post: 01-03-2009, 03:56 AM
  2. Help printing a multi array
    By cjohnman in forum C Programming
    Replies: 4
    Last Post: 05-05-2008, 02:35 PM
  3. multi threaded program
    By Dash_Riprock in forum C++ Programming
    Replies: 6
    Last Post: 08-27-2006, 09:38 AM
  4. Replies: 6
    Last Post: 04-26-2004, 11:02 PM
  5. Signals in multi threaded application
    By HelpMe in forum C Programming
    Replies: 0
    Last Post: 09-12-2001, 10:15 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21